Re: [PATCH] [v2] media: imx: imx7-mipi-csis: Fix runtime PM imbalance in mipi_csis_s_stream

[Rui Miguel Silva]

Hi Liu,
Many thanks for the patch.

On Fri, Apr 09, 2021 at 04:22:25PM +0800, Dinghao Liu wrote:
> When v4l2_subdev_call() fails, a pairing PM usage counter
> decrement is needed to keep the counter balanced. It's the
> same for the following error paths in case 'enable' is on.
> 
> Signed-off-by: Dinghao Liu 

LGTM
Reviewed-by: Rui Miguel Silva 

--
Cheers,
 Rui
> ---
> 
> Changelog:
> 
> v2: - Use pm_runtime_put() to balance the refcount.
> ---
>  drivers/staging/media/imx/imx7-mipi-csis.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c 
> b/drivers/staging/media/imx/imx7-mipi-csis.c
> index a01a7364b4b9..6f3e8a15e7c4 100644
> --- a/drivers/staging/media/imx/imx7-mipi-csis.c
> +++ b/drivers/staging/media/imx/imx7-mipi-csis.c
> @@ -628,7 +628,7 @@ static int mipi_csis_s_stream(struct v4l2_subdev 
> *mipi_sd, int enable)
>   }
>   ret = v4l2_subdev_call(state->src_sd, core, s_power, 1);
>   if (ret < 0)
> - return ret;
> + goto pm_put;
>   }
>  
>   mutex_lock(>lock);
> @@ -657,7 +657,8 @@ static int mipi_csis_s_stream(struct v4l2_subdev 
> *mipi_sd, int enable)
>  
>  unlock:
>   mutex_unlock(>lock);
> - if (!enable)
> +pm_put:
> + if (!enable || (ret < 0))
>   pm_runtime_put(>pdev->dev);
>  
>   return ret;
> -- 
> 2.17.1
> 
> 

Re: [PATCH] media: imx: imx7-mipi-csis: Fix runtime PM imbalance in mipi_csis_s_stream

[Rui Miguel Silva]

Hi Liu,
Thanks for your patch.

On Thu, Apr 08, 2021 at 05:08:27PM +0800, Dinghao Liu wrote:
> When v4l2_subdev_call() fails, a pairing PM usage counter
> decrement is needed to keep the counter balanced. It's the
> same for the following error paths in case 'enable' is on.
> 
> Signed-off-by: Dinghao Liu 
> ---
>  drivers/staging/media/imx/imx7-mipi-csis.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c 
> b/drivers/staging/media/imx/imx7-mipi-csis.c
> index a01a7364b4b9..2a3fff231a40 100644
> --- a/drivers/staging/media/imx/imx7-mipi-csis.c
> +++ b/drivers/staging/media/imx/imx7-mipi-csis.c
> @@ -627,21 +627,26 @@ static int mipi_csis_s_stream(struct v4l2_subdev 
> *mipi_sd, int enable)
>   return ret;
>   }
>   ret = v4l2_subdev_call(state->src_sd, core, s_power, 1);
> - if (ret < 0)
> + if (ret < 0) {
> + pm_runtime_put_noidle(>pdev->dev);

I think here we should go completely pm_runtime_put to call the
mipi_csis_pm_suspend down the line, right?

>   return ret;
> + }
>   }
>  
>   mutex_lock(>lock);
>   if (enable) {
>   if (state->flags & ST_SUSPENDED) {
>   ret = -EBUSY;
> + pm_runtime_put_noidle(>pdev->dev);

since we are in ST_SUSPENDED state, for sure the pm counter was
already 0.

>   goto unlock;
>   }
>  
>   mipi_csis_start_stream(state);
>   ret = v4l2_subdev_call(state->src_sd, video, s_stream, 1);
> - if (ret < 0)
> + if (ret < 0) {
> + pm_runtime_put_noidle(>pdev->dev);

here also we need the pm_runtime_put, maybe just changing the unlock
tag bellow from:
if (!enable)
pm_runtime_put(>pdev->dev);

to 
if (!enable || (ret < 0))
pm_runtime_put(>pdev->dev);

will not hurt the first case and will complete the suspend routine
afterward in the second case.

--
Cheers,
 Rui
>   goto unlock;
> + }
>  
>   mipi_csis_log_counters(state, true);
>  
> -- 
> 2.17.1
> 
> 

Re: [PATCH 23/32] MAINTAINERS: update ovti,ov2680.yaml reference

[Rui Miguel Silva]

Hi Mauro,
Thanks for fixing this.

On Thu, Apr 01, 2021 at 02:17:43PM +0200, Mauro Carvalho Chehab wrote:
> The file name: Documentation/devicetree/bindings/media/i2c/ov2680.yaml
> should be, instead: 
> Documentation/devicetree/bindings/media/i2c/ovti,ov2680.yaml.
> 
> Update its cross-reference accordingly.
> 
> Fixes: 57226cd8c8bf ("media: dt-bindings: ov2680: convert bindings to yaml")
> Signed-off-by: Mauro Carvalho Chehab 

Reviewed-by: Rui Miguel Silva 

--
Cheers,
 Rui
> ---
>  MAINTAINERS | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0ee42d68a269..1644b6e9697c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13351,7 +13351,7 @@ M:Rui Miguel Silva 
>  L:   linux-me...@vger.kernel.org
>  S:   Maintained
>  T:   git git://linuxtv.org/media_tree.git
> -F:   Documentation/devicetree/bindings/media/i2c/ov2680.yaml
> +F:   Documentation/devicetree/bindings/media/i2c/ovti,ov2680.yaml
>  F:   drivers/media/i2c/ov2680.c
>  
>  OMNIVISION OV2685 SENSOR DRIVER
> -- 
> 2.30.2
> 

Re: [PATCH 06/10] staging: greybus: spilib: use 'spi_delay_to_ns' for getting xfer delay

[Rui Miguel Silva]

Hi,
On Tue, Mar 09, 2021 at 09:58:09AM +0530, Viresh Kumar wrote:
> On 08-03-21, 16:54, Alexandru Ardelean wrote:
> > The intent is the removal of the 'delay_usecs' field from the
> > spi_transfer struct, as there is a 'delay' field that does the same
> > thing.
> > 
> > The spi_delay_to_ns() can be used to get the transfer delay. It works by
> > using the 'delay_usecs' field first (if it is non-zero), and finally
> > uses the 'delay' field.
> > 
> > Since the 'delay_usecs' field is going away, this change makes use of the
> > spi_delay_to_ns() function. This also means dividing the return value of
> > the function by 1000, to convert it to microseconds.
> > To prevent any potential faults when converting to microseconds and since
> > the result of spi_delay_to_ns() is int, the delay is being computed in 32
> > bits and then clamped between 0 & U16_MAX.
> > 
> > Signed-off-by: Alexandru Ardelean 
> > ---
> >  drivers/staging/greybus/spilib.c | 5 -
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/greybus/spilib.c 
> > b/drivers/staging/greybus/spilib.c
> > index 672d540d3365..30655153df6a 100644
> > --- a/drivers/staging/greybus/spilib.c
> > +++ b/drivers/staging/greybus/spilib.c
> > @@ -245,6 +245,7 @@ static struct gb_operation 
> > *gb_spi_operation_create(struct gb_spilib *spi,
> > /* Fill in the transfers array */
> > xfer = spi->first_xfer;
> > while (msg->state != GB_SPI_STATE_OP_DONE) {
> > +   int xfer_delay;
> > if (xfer == spi->last_xfer)
> > xfer_len = spi->last_xfer_size;
> > else
> > @@ -259,7 +260,9 @@ static struct gb_operation 
> > *gb_spi_operation_create(struct gb_spilib *spi,
> >  
> > gb_xfer->speed_hz = cpu_to_le32(xfer->speed_hz);
> > gb_xfer->len = cpu_to_le32(xfer_len);
> > -   gb_xfer->delay_usecs = cpu_to_le16(xfer->delay_usecs);
> > +   xfer_delay = spi_delay_to_ns(>delay, xfer) / 1000;
> > +       xfer_delay = clamp_t(u16, xfer_delay, 0, U16_MAX);
> > +   gb_xfer->delay_usecs = cpu_to_le16(xfer_delay);
> > gb_xfer->cs_change = xfer->cs_change;
> > gb_xfer->bits_per_word = xfer->bits_per_word;
> 
> Acked-by: Viresh Kumar 

Acked-by: Rui Miguel Silva 

--
Cheers,
 Rui

Re: [PATCH v2 -next] staging: greybus: light: Use kzalloc for allocating only one thing

[Rui Miguel Silva]

Hi Yongjun,
many thanks for your patch.

On Wed, Dec 30, 2020 at 09:37:06AM +0800, Zheng Yongjun wrote:
> Use kzalloc rather than kcalloc(1,...)
> 
> The semantic patch that makes this change is as follows:
> (http://coccinelle.lip6.fr/)
> 
> // 
> @@
> @@
> 
> - kcalloc(1,
> + kzalloc(
>   ...)
> // 
> 
> Signed-off-by: Zheng Yongjun 

Like Alex said, LGTM also.

Reviewed-by: Rui Miguel Silva 

--
Cheers,
 Rui
> ---
>  drivers/staging/greybus/light.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c
> index d2672b65c3f4..87d36948c610 100644
> --- a/drivers/staging/greybus/light.c
> +++ b/drivers/staging/greybus/light.c
> @@ -290,8 +290,7 @@ static int channel_attr_groups_set(struct gb_channel 
> *channel,
>   channel->attrs = kcalloc(size + 1, sizeof(*channel->attrs), GFP_KERNEL);
>   if (!channel->attrs)
>   return -ENOMEM;
> - channel->attr_group = kcalloc(1, sizeof(*channel->attr_group),
> -   GFP_KERNEL);
> + channel->attr_group = kzalloc(sizeof(*channel->attr_group), GFP_KERNEL);
>   if (!channel->attr_group)
>   return -ENOMEM;
>   channel->attr_groups = kcalloc(2, sizeof(*channel->attr_groups),
> -- 
> 2.22.0
> 

[PATCH] optee: add writeback to valid memory type

[Rui Miguel Silva]

Only in smp systems the cache policy is setup as write alloc, in
single cpu systems the cache policy is set as writeback and it is
normal memory, so, it should pass the is_normal_memory check in the
share memory registration.

Add the right condition to make it work in no smp systems.

Fixes: cdbcf83d29c1 ("tee: optee: check type of registered shared memory")
Signed-off-by: Rui Miguel Silva 

---
 drivers/tee/optee/call.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/tee/optee/call.c b/drivers/tee/optee/call.c
index 20b6fd7383c5..c981757ba0d4 100644
--- a/drivers/tee/optee/call.c
+++ b/drivers/tee/optee/call.c
@@ -534,7 +534,8 @@ void optee_free_pages_list(void *list, size_t num_entries)
 static bool is_normal_memory(pgprot_t p)
 {
 #if defined(CONFIG_ARM)
-   return (pgprot_val(p) & L_PTE_MT_MASK) == L_PTE_MT_WRITEALLOC;
+   return (((pgprot_val(p) & L_PTE_MT_MASK) == L_PTE_MT_WRITEALLOC) ||
+   ((pgprot_val(p) & L_PTE_MT_MASK) == L_PTE_MT_WRITEBACK));
 #elif defined(CONFIG_ARM64)
return (pgprot_val(p) & PTE_ATTRINDX_MASK) == PTE_ATTRINDX(MT_NORMAL);
 #else
-- 
2.29.2

Re: [PATCH 18/30] iio: gyro: fxas21002c: Move 'fxas21002c_reg_fields' to the only file its used

[Rui Miguel Silva]

Hi Lee,
Sorry for the late reply, been out of office and catching up with
mails now.
On Fri, Jul 17, 2020 at 05:55:26PM +0100, Lee Jones wrote:
> 'fxas21002c_reg_fields' is only used in '*core*', meaning that '*i2c*'
> and '*spi*' complain of a defined but not used const variable.  Let's
> move it into the source file.
> 
> Fixes the following W=1 kernel build warning(s):
> 
>  In file included from drivers/iio/gyro/fxas21002c_i2c.c:14:
>  drivers/iio/gyro/fxas21002c.h:79:31: warning: ‘fxas21002c_reg_fields’ 
> defined but not used [-Wunused-const-variable=]
>  79 | static const struct reg_field fxas21002c_reg_fields[] = {
>  | ^
>  In file included from drivers/iio/gyro/fxas21002c_spi.c:14:
>  drivers/iio/gyro/fxas21002c.h:79:31: warning: ‘fxas21002c_reg_fields’ 
> defined but not used [-Wunused-const-variable=]
>  79 | static const struct reg_field fxas21002c_reg_fields[] = {
>  | ^
> 
> Cc: Rui Miguel Silva 
>

Thanks for this, not going in time for the official tagging, since
gregkh already picked this up for staging, but fwiw:

Acked-by: Rui Miguel Silva 

--
Cheers,
 Rui

> Signed-off-by: Lee Jones 
> ---
>  drivers/iio/gyro/fxas21002c.h  | 66 --
>  drivers/iio/gyro/fxas21002c_core.c | 66 ++
>  2 files changed, 66 insertions(+), 66 deletions(-)
> 
> diff --git a/drivers/iio/gyro/fxas21002c.h b/drivers/iio/gyro/fxas21002c.h
> index 566d92de26763..c81cecee121cb 100644
> --- a/drivers/iio/gyro/fxas21002c.h
> +++ b/drivers/iio/gyro/fxas21002c.h
> @@ -76,72 +76,6 @@ enum fxas21002c_fields {
>   F_MAX_FIELDS,
>  };
>  
> -static const struct reg_field fxas21002c_reg_fields[] = {
> - [F_DR_STATUS]   = REG_FIELD(FXAS21002C_REG_STATUS, 0, 7),
> - [F_OUT_X_MSB]   = REG_FIELD(FXAS21002C_REG_OUT_X_MSB, 0, 7),
> - [F_OUT_X_LSB]   = REG_FIELD(FXAS21002C_REG_OUT_X_LSB, 0, 7),
> - [F_OUT_Y_MSB]   = REG_FIELD(FXAS21002C_REG_OUT_Y_MSB, 0, 7),
> - [F_OUT_Y_LSB]   = REG_FIELD(FXAS21002C_REG_OUT_Y_LSB, 0, 7),
> - [F_OUT_Z_MSB]   = REG_FIELD(FXAS21002C_REG_OUT_Z_MSB, 0, 7),
> - [F_OUT_Z_LSB]   = REG_FIELD(FXAS21002C_REG_OUT_Z_LSB, 0, 7),
> - [F_ZYX_OW]  = REG_FIELD(FXAS21002C_REG_DR_STATUS, 7, 7),
> - [F_Z_OW]= REG_FIELD(FXAS21002C_REG_DR_STATUS, 6, 6),
> - [F_Y_OW]= REG_FIELD(FXAS21002C_REG_DR_STATUS, 5, 5),
> - [F_X_OW]= REG_FIELD(FXAS21002C_REG_DR_STATUS, 4, 4),
> - [F_ZYX_DR]  = REG_FIELD(FXAS21002C_REG_DR_STATUS, 3, 3),
> - [F_Z_DR]= REG_FIELD(FXAS21002C_REG_DR_STATUS, 2, 2),
> - [F_Y_DR]= REG_FIELD(FXAS21002C_REG_DR_STATUS, 1, 1),
> - [F_X_DR]= REG_FIELD(FXAS21002C_REG_DR_STATUS, 0, 0),
> - [F_OVF] = REG_FIELD(FXAS21002C_REG_F_STATUS, 7, 7),
> - [F_WMKF]= REG_FIELD(FXAS21002C_REG_F_STATUS, 6, 6),
> - [F_CNT] = REG_FIELD(FXAS21002C_REG_F_STATUS, 0, 5),
> - [F_MODE]= REG_FIELD(FXAS21002C_REG_F_SETUP, 6, 7),
> - [F_WMRK]= REG_FIELD(FXAS21002C_REG_F_SETUP, 0, 5),
> - [F_EVENT]   = REG_FIELD(FXAS21002C_REG_F_EVENT, 5, 5),
> - [FE_TIME]   = REG_FIELD(FXAS21002C_REG_F_EVENT, 0, 4),
> - [F_BOOTEND] = REG_FIELD(FXAS21002C_REG_INT_SRC_FLAG, 3, 3),
> - [F_SRC_FIFO]= REG_FIELD(FXAS21002C_REG_INT_SRC_FLAG, 2, 2),
> - [F_SRC_RT]  = REG_FIELD(FXAS21002C_REG_INT_SRC_FLAG, 1, 1),
> - [F_SRC_DRDY]= REG_FIELD(FXAS21002C_REG_INT_SRC_FLAG, 0, 0),
> - [F_WHO_AM_I]= REG_FIELD(FXAS21002C_REG_WHO_AM_I, 0, 7),
> - [F_BW]  = REG_FIELD(FXAS21002C_REG_CTRL0, 6, 7),
> - [F_SPIW]= REG_FIELD(FXAS21002C_REG_CTRL0, 5, 5),
> - [F_SEL] = REG_FIELD(FXAS21002C_REG_CTRL0, 3, 4),
> - [F_HPF_EN]  = REG_FIELD(FXAS21002C_REG_CTRL0, 2, 2),
> - [F_FS]  = REG_FIELD(FXAS21002C_REG_CTRL0, 0, 1),
> - [F_ELE] = REG_FIELD(FXAS21002C_REG_RT_CFG, 3, 3),
> - [F_ZTEFE]   = REG_FIELD(FXAS21002C_REG_RT_CFG, 2, 2),
> - [F_YTEFE]   = REG_FIELD(FXAS21002C_REG_RT_CFG, 1, 1),
> - [F_XTEFE]   = REG_FIELD(FXAS21002C_REG_RT_CFG, 0, 0),
> - [F_EA]  = REG_FIELD(FXAS21002C_REG_RT_SRC, 6, 6),
> - [F_ZRT] = REG_FIELD(FXAS21002C_REG_RT_SRC, 5, 5),
> - [F_ZRT_POL] = REG_FIELD(FXAS21002C_REG_RT_SRC, 4, 4),
> - [F_YRT] = REG_FIELD(FXAS21002C_REG_RT_SRC, 3, 3),
> - [F_YRT

Re: [PATCH v8 5/5] media: imx: Try colorimetry at both sink and source pads

[Rui Miguel Silva]

Hi Steve,
On Tue 22 Oct 2019 at 17:26, Steve Longerbeam wrote:
> Hi Laurent, Rui,
>
> Besides field type ANY, the imx7 CSI should probably be dealing with other 
> field
> type conversions as well. I may be mistaken, but like the imx6, the imx7 does
> not have the ability to detect whether a captured field is a top field or a
> bottom field, so it can't support ALTERNATE mode. It should probably capture 
> two
> consecutive fields and in that case and report seq-tb or seq-bt at its output.
> Also the imx6 supports interlacing field lines, if that is the case for imx7 
> it
> should also support converting ALTERNATE to INTERLACED at its output.
>

Yeah, that makes sense to me, I already saw yours csi_try_field
that does something in this lines.

I will try to handle that in imx7 also.

Thanks for your inputs here.

Cheers,
Rui
>
> Steve
>
>
> On 10/22/19 6:34 AM, Rui Miguel Silva wrote:
>> Hi Laurent,
>> On Tue 22 Oct 2019 at 02:44, Laurent Pinchart wrote:
>>> Hi Steve,
>>>
>>> On Tue, May 21, 2019 at 06:03:17PM -0700, Steve Longerbeam wrote:
>>>> Retask imx_media_fill_default_mbus_fields() to try colorimetry parameters,
>>>> renaming it to to imx_media_try_colorimetry(), and call it at both sink and
>>>> source pad try_fmt's. The unrelated check for uninitialized field value is
>>>> moved out to appropriate places in each subdev try_fmt.
>>>>
>>>> The IC now supports Rec.709 and BT.601 Y'CbCr encoding, and both limited
>>>> and full range quantization for both YUV and RGB space, so allow those
>>>> for pipelines that route through the IC.
>>>>
>>>> Signed-off-by: Steve Longerbeam 
>>>> ---
>>>> Changes in v7:
>>>> - squashed with "media: imx: Allow Rec.709 encoding for IC routes".
>>>> - remove the RGB full-range quantization restriction for IC routes.
>>>> ---
>>>>   drivers/staging/media/imx/imx-ic-prp.c  |  6 +-
>>>>   drivers/staging/media/imx/imx-ic-prpencvf.c |  8 +--
>>>>   drivers/staging/media/imx/imx-media-csi.c   | 19 +++---
>>>>   drivers/staging/media/imx/imx-media-utils.c | 73 ++---
>>>>   drivers/staging/media/imx/imx-media-vdic.c  |  5 +-
>>>>   drivers/staging/media/imx/imx-media.h   |  5 +-
>>>>   drivers/staging/media/imx/imx7-media-csi.c  |  8 +--
>>>>   7 files changed, 62 insertions(+), 62 deletions(-)
>>>>
>>>> diff --git a/drivers/staging/media/imx/imx-ic-prp.c 
>>>> b/drivers/staging/media/imx/imx-ic-prp.c
>>>> index 10ffe00f1a54..f87fe0203720 100644
>>>> --- a/drivers/staging/media/imx/imx-ic-prp.c
>>>> +++ b/drivers/staging/media/imx/imx-ic-prp.c
>>>> @@ -193,8 +193,8 @@ static int prp_set_fmt(struct v4l2_subdev *sd,
>>>>sdformat->format.code = cc->codes[0];
>>>>}
>>>>
>>>> -  imx_media_fill_default_mbus_fields(>format, infmt,
>>>> - true);
>>>> +  if (sdformat->format.field == V4L2_FIELD_ANY)
>>>> +  sdformat->format.field = V4L2_FIELD_NONE;
>>>>break;
>>>>case PRP_SRC_PAD_PRPENC:
>>>>case PRP_SRC_PAD_PRPVF:
>>>> @@ -203,6 +203,8 @@ static int prp_set_fmt(struct v4l2_subdev *sd,
>>>>break;
>>>>}
>>>>
>>>> +  imx_media_try_colorimetry(>format, true);
>>>> +
>>>>fmt = __prp_get_fmt(priv, cfg, sdformat->pad, sdformat->which);
>>>>*fmt = sdformat->format;
>>>>   out:
>>>> diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c 
>>>> b/drivers/staging/media/imx/imx-ic-prpencvf.c
>>>> index e8b36a181ccc..f2fe3c11c70e 100644
>>>> --- a/drivers/staging/media/imx/imx-ic-prpencvf.c
>>>> +++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
>>>> @@ -907,8 +907,6 @@ static void prp_try_fmt(struct prp_priv *priv,
>>>>/* propagate colorimetry from sink */
>>>>sdformat->format.colorspace = infmt->colorspace;
>>>>sdformat->format.xfer_func = infmt->xfer_func;
>>>> -  sdformat->format.quantization = infmt->quantization;
>>>> -  sdformat->format.ycbcr_enc = infmt->ycbcr_enc;
>>>>} else {
>>>>v4l_bound_align_image(>format.width,
>>>> 

Re: [PATCH v8 5/5] media: imx: Try colorimetry at both sink and source pads

[Rui Miguel Silva]

Hi Laurent,
On Tue 22 Oct 2019 at 02:44, Laurent Pinchart wrote:
> Hi Steve,
>
> On Tue, May 21, 2019 at 06:03:17PM -0700, Steve Longerbeam wrote:
>> Retask imx_media_fill_default_mbus_fields() to try colorimetry parameters,
>> renaming it to to imx_media_try_colorimetry(), and call it at both sink and
>> source pad try_fmt's. The unrelated check for uninitialized field value is
>> moved out to appropriate places in each subdev try_fmt.
>>
>> The IC now supports Rec.709 and BT.601 Y'CbCr encoding, and both limited
>> and full range quantization for both YUV and RGB space, so allow those
>> for pipelines that route through the IC.
>>
>> Signed-off-by: Steve Longerbeam 
>> ---
>> Changes in v7:
>> - squashed with "media: imx: Allow Rec.709 encoding for IC routes".
>> - remove the RGB full-range quantization restriction for IC routes.
>> ---
>>  drivers/staging/media/imx/imx-ic-prp.c  |  6 +-
>>  drivers/staging/media/imx/imx-ic-prpencvf.c |  8 +--
>>  drivers/staging/media/imx/imx-media-csi.c   | 19 +++---
>>  drivers/staging/media/imx/imx-media-utils.c | 73 ++---
>>  drivers/staging/media/imx/imx-media-vdic.c  |  5 +-
>>  drivers/staging/media/imx/imx-media.h   |  5 +-
>>  drivers/staging/media/imx/imx7-media-csi.c  |  8 +--
>>  7 files changed, 62 insertions(+), 62 deletions(-)
>>
>> diff --git a/drivers/staging/media/imx/imx-ic-prp.c 
>> b/drivers/staging/media/imx/imx-ic-prp.c
>> index 10ffe00f1a54..f87fe0203720 100644
>> --- a/drivers/staging/media/imx/imx-ic-prp.c
>> +++ b/drivers/staging/media/imx/imx-ic-prp.c
>> @@ -193,8 +193,8 @@ static int prp_set_fmt(struct v4l2_subdev *sd,
>>  sdformat->format.code = cc->codes[0];
>>  }
>>
>> -imx_media_fill_default_mbus_fields(>format, infmt,
>> -   true);
>> +if (sdformat->format.field == V4L2_FIELD_ANY)
>> +sdformat->format.field = V4L2_FIELD_NONE;
>>  break;
>>  case PRP_SRC_PAD_PRPENC:
>>  case PRP_SRC_PAD_PRPVF:
>> @@ -203,6 +203,8 @@ static int prp_set_fmt(struct v4l2_subdev *sd,
>>  break;
>>  }
>>
>> +imx_media_try_colorimetry(>format, true);
>> +
>>  fmt = __prp_get_fmt(priv, cfg, sdformat->pad, sdformat->which);
>>  *fmt = sdformat->format;
>>  out:
>> diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c 
>> b/drivers/staging/media/imx/imx-ic-prpencvf.c
>> index e8b36a181ccc..f2fe3c11c70e 100644
>> --- a/drivers/staging/media/imx/imx-ic-prpencvf.c
>> +++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
>> @@ -907,8 +907,6 @@ static void prp_try_fmt(struct prp_priv *priv,
>>  /* propagate colorimetry from sink */
>>  sdformat->format.colorspace = infmt->colorspace;
>>  sdformat->format.xfer_func = infmt->xfer_func;
>> -sdformat->format.quantization = infmt->quantization;
>> -sdformat->format.ycbcr_enc = infmt->ycbcr_enc;
>>  } else {
>>  v4l_bound_align_image(>format.width,
>>MIN_W_SINK, MAX_W_SINK, W_ALIGN_SINK,
>> @@ -916,9 +914,11 @@ static void prp_try_fmt(struct prp_priv *priv,
>>MIN_H_SINK, MAX_H_SINK, H_ALIGN_SINK,
>>S_ALIGN);
>>
>> -imx_media_fill_default_mbus_fields(>format, infmt,
>> -   true);
>> +if (sdformat->format.field == V4L2_FIELD_ANY)
>> +sdformat->format.field = V4L2_FIELD_NONE;
>>  }
>> +
>> +imx_media_try_colorimetry(>format, true);
>>  }
>>
>>  static int prp_set_fmt(struct v4l2_subdev *sd,
>> diff --git a/drivers/staging/media/imx/imx-media-csi.c 
>> b/drivers/staging/media/imx/imx-media-csi.c
>> index 1d248aca40a9..dce4addadff4 100644
>> --- a/drivers/staging/media/imx/imx-media-csi.c
>> +++ b/drivers/staging/media/imx/imx-media-csi.c
>> @@ -1375,9 +1375,15 @@ static void csi_try_field(struct csi_priv *priv,
>>  struct v4l2_mbus_framefmt *infmt =
>>  __csi_get_fmt(priv, cfg, CSI_SINK_PAD, sdformat->which);
>>
>> -/* no restrictions on sink pad field type */
>> -if (sdformat->pad == CSI_SINK_PAD)
>> +/*
>> + * no restrictions on sink pad field type except must
>> + * be initialized.
>> + */
>> +if (sdformat->pad == CSI_SINK_PAD) {
>> +if (sdformat->format.field == V4L2_FIELD_ANY)
>> +sdformat->format.field = V4L2_FIELD_NONE;
>>  return;
>> +}
>>
>>  switch (infmt->field) {
>>  case V4L2_FIELD_SEQ_TB:
>> @@ -1455,8 +1461,6 @@ static void csi_try_fmt(struct csi_priv *priv,
>>  /* propagate colorimetry from sink */
>>  sdformat->format.colorspace = infmt->colorspace;
>>  sdformat->format.xfer_func = infmt->xfer_func;
>> -sdformat->format.quantization = infmt->quantization;
>> -sdformat->format.ycbcr_enc = 

Re: [PATCH v2] media: imx7-mipi-csis: Add a check for devm_regulator_get

[Rui Miguel Silva]

Hi Chuhong,
many thanks for the patch.

On Tue 15 Oct 2019 at 14:59, Chuhong Yuan wrote:
> devm_regulator_get may return an error but mipi_csis_phy_init misses
> a check for it.
> This may lead to problems when regulator_set_voltage uses the unchecked
> pointer.
> This patch adds a check for devm_regulator_get to avoid potential risk.
>
> Signed-off-by: Chuhong Yuan 

Reviewed-by: Rui Miguel Silva 

---
Cheers,
Rui

> ---
> Changes in v2:
>   - Add a check in mipi_csis_probe for the modified mipi_csis_phy_init.
>
>  drivers/staging/media/imx/imx7-mipi-csis.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c 
> b/drivers/staging/media/imx/imx7-mipi-csis.c
> index 73d8354e618c..e8a6acaa969e 100644
> --- a/drivers/staging/media/imx/imx7-mipi-csis.c
> +++ b/drivers/staging/media/imx/imx7-mipi-csis.c
> @@ -350,6 +350,8 @@ static void mipi_csis_sw_reset(struct csi_state *state)
>  static int mipi_csis_phy_init(struct csi_state *state)
>  {
>   state->mipi_phy_regulator = devm_regulator_get(state->dev, "phy");
> + if (IS_ERR(state->mipi_phy_regulator))
> + return PTR_ERR(state->mipi_phy_regulator);
>
>   return regulator_set_voltage(state->mipi_phy_regulator, 100,
>100);
> @@ -966,7 +968,10 @@ static int mipi_csis_probe(struct platform_device *pdev)
>   return ret;
>   }
>
> - mipi_csis_phy_init(state);
> + ret = mipi_csis_phy_init(state);
> + if (ret < 0)
> + return ret;
> +
>   mipi_csis_phy_reset(state);
>
>   mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);

Re: [PATCH v2] media: imx7-mipi-csis: Add a check for devm_regulator_get

[Rui Miguel Silva]

Hi Marco,
On Thu 17 Oct 2019 at 09:10, Marco Felsch wrote:
> Hi Rui,
>
> On 19-10-16 14:43, Rui Miguel Silva wrote:
>> Hi Marco,
>> On Wed 16 Oct 2019 at 10:06, Marco Felsch wrote:
>> > Hi Chuhong,
>> >
>> > On 19-10-15 21:59, Chuhong Yuan wrote:
>> >> devm_regulator_get may return an error but mipi_csis_phy_init misses
>> >> a check for it.
>> >> This may lead to problems when regulator_set_voltage uses the unchecked
>> >> pointer.
>> >> This patch adds a check for devm_regulator_get to avoid potential risk.
>> >>
>> >> Signed-off-by: Chuhong Yuan 
>> >> ---
>> >> Changes in v2:
>> >>   - Add a check in mipi_csis_probe for the modified mipi_csis_phy_init.
>> >
>> > Did you miss the check for -EPROBE_DEFER?
>> >
>>
>> I think nothing special is really needed to do in case of
>> EPROBE_DEFER, or am I missing something?
>> It just return to probe and probe returns also. I just talked
>> about it because it was not cover in the original code.
>
> Yes, your are right... I shouldn't comment on anything I read with one
> eye. Sorry.
>

ehehe, no problem and thanks for your inputs.

---
Cheers,
Rui

>
> Regards,
>   Marco
>
>> ---
>> Cheers,
>>  Rui
>>
>> >
>> > Regards,
>> >   Marco
>> >
>> >>
>> >>  drivers/staging/media/imx/imx7-mipi-csis.c | 8 +++-
>> >>  1 file changed, 7 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c 
>> >> b/drivers/staging/media/imx/imx7-mipi-csis.c
>> >> index 73d8354e618c..e8a6acaa969e 100644
>> >> --- a/drivers/staging/media/imx/imx7-mipi-csis.c
>> >> +++ b/drivers/staging/media/imx/imx7-mipi-csis.c
>> >> @@ -350,6 +350,8 @@ static void mipi_csis_sw_reset(struct csi_state 
>> >> *state)
>> >>  static int mipi_csis_phy_init(struct csi_state *state)
>> >>  {
>> >>   state->mipi_phy_regulator = devm_regulator_get(state->dev, "phy");
>> >> + if (IS_ERR(state->mipi_phy_regulator))
>> >> + return PTR_ERR(state->mipi_phy_regulator);
>> >>
>> >>   return regulator_set_voltage(state->mipi_phy_regulator, 100,
>> >>100);
>> >> @@ -966,7 +968,10 @@ static int mipi_csis_probe(struct platform_device 
>> >> *pdev)
>> >>   return ret;
>> >>   }
>> >>
>> >> - mipi_csis_phy_init(state);
>> >> + ret = mipi_csis_phy_init(state);
>> >> + if (ret < 0)
>> >> + return ret;
>> >> +
>> >>   mipi_csis_phy_reset(state);
>> >>
>> >>   mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> >> --
>> >> 2.20.1
>> >>
>> >>
>> >>
>>
>>

Re: [PATCH v2] media: imx7-mipi-csis: Add a check for devm_regulator_get

[Rui Miguel Silva]

Hi Marco,
On Wed 16 Oct 2019 at 10:06, Marco Felsch wrote:
> Hi Chuhong,
>
> On 19-10-15 21:59, Chuhong Yuan wrote:
>> devm_regulator_get may return an error but mipi_csis_phy_init misses
>> a check for it.
>> This may lead to problems when regulator_set_voltage uses the unchecked
>> pointer.
>> This patch adds a check for devm_regulator_get to avoid potential risk.
>>
>> Signed-off-by: Chuhong Yuan 
>> ---
>> Changes in v2:
>>   - Add a check in mipi_csis_probe for the modified mipi_csis_phy_init.
>
> Did you miss the check for -EPROBE_DEFER?
>

I think nothing special is really needed to do in case of
EPROBE_DEFER, or am I missing something?
It just return to probe and probe returns also. I just talked
about it because it was not cover in the original code.

---
Cheers,
Rui

>
> Regards,
>   Marco
>
>>
>>  drivers/staging/media/imx/imx7-mipi-csis.c | 8 +++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c 
>> b/drivers/staging/media/imx/imx7-mipi-csis.c
>> index 73d8354e618c..e8a6acaa969e 100644
>> --- a/drivers/staging/media/imx/imx7-mipi-csis.c
>> +++ b/drivers/staging/media/imx/imx7-mipi-csis.c
>> @@ -350,6 +350,8 @@ static void mipi_csis_sw_reset(struct csi_state *state)
>>  static int mipi_csis_phy_init(struct csi_state *state)
>>  {
>>  state->mipi_phy_regulator = devm_regulator_get(state->dev, "phy");
>> +if (IS_ERR(state->mipi_phy_regulator))
>> +return PTR_ERR(state->mipi_phy_regulator);
>>
>>  return regulator_set_voltage(state->mipi_phy_regulator, 100,
>>   100);
>> @@ -966,7 +968,10 @@ static int mipi_csis_probe(struct platform_device *pdev)
>>  return ret;
>>  }
>>
>> -mipi_csis_phy_init(state);
>> +ret = mipi_csis_phy_init(state);
>> +if (ret < 0)
>> +return ret;
>> +
>>  mipi_csis_phy_reset(state);
>>
>>  mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> --
>> 2.20.1
>>
>>
>>

Re: [PATCH] media: imx7-mipi-csis: Add a check for devm_regulator_get

[Rui Miguel Silva]

Hi Chuhong,
Thanks for the patch.

On Mon 14 Oct 2019 at 03:08, Chuhong Yuan wrote:
> devm_regulator_get may return an error but mipi_csis_phy_init misses
> a check for it.
> This may lead to problems when regulator_set_voltage uses the unchecked
> pointer.
> This patch adds a check for devm_regulator_get to avoid potential risk.
>
> Signed-off-by: Chuhong Yuan 
> ---
>  drivers/staging/media/imx/imx7-mipi-csis.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c 
> b/drivers/staging/media/imx/imx7-mipi-csis.c
> index 73d8354e618c..9a07b54c4ab1 100644
> --- a/drivers/staging/media/imx/imx7-mipi-csis.c
> +++ b/drivers/staging/media/imx/imx7-mipi-csis.c
> @@ -350,6 +350,8 @@ static void mipi_csis_sw_reset(struct csi_state *state)
>  static int mipi_csis_phy_init(struct csi_state *state)
>  {
>   state->mipi_phy_regulator = devm_regulator_get(state->dev, "phy");
> + if (IS_ERR(state->mipi_phy_regulator))
> + return PTR_ERR(state->mipi_phy_regulator);

This regulator is marked as mandatory in the device tree entry,
however it looks good to me to have this check, even because it
can return -EPROBE_DEFER and we need to retry.

But for that we may need to extend this patch to make the caller
of this (mipi_csis_probe), to also really care about the returned
code.

Cheers,
   Rui

>
>   return regulator_set_voltage(state->mipi_phy_regulator, 100,
>100);

Re: [PATCH] staging: media: imx: make use devm_platform_ioremap_resource

[Rui Miguel Silva]

Hi Hariprasad,
Thanks for the patch
On Tue 08 Oct 2019 at 07:17, nobody wrote:
> From: Hariprasad Kelam 
>

Something went wrong formating the patch email, no To: nor From:

>
> fix below issue reported by coccicheck
> drivers/staging//media/imx/imx7-mipi-csis.c:973:1-12: WARNING: Use
> devm_platform_ioremap_resource for state -> regs
>

Sorry, but someone else, Jeeeun, already sent a patch for this
[0]. Thanks anyway.

---
Cheers,
Rui


[0]: https://lore.kernel.org/linux-media/m3wodvgec4@gmail.com/

>
> Signed-off-by: Hariprasad Kelam 
> ---
>  drivers/staging/media/imx/imx7-mipi-csis.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c 
> b/drivers/staging/media/imx/imx7-mipi-csis.c
> index 73d8354..bf21db3 100644
> --- a/drivers/staging/media/imx/imx7-mipi-csis.c
> +++ b/drivers/staging/media/imx/imx7-mipi-csis.c
> @@ -947,7 +947,6 @@ static void mipi_csis_debugfs_exit(struct csi_state 
> *state)
>  static int mipi_csis_probe(struct platform_device *pdev)
>  {
>   struct device *dev = >dev;
> - struct resource *mem_res;
>   struct csi_state *state;
>   int ret;
>
> @@ -969,8 +968,7 @@ static int mipi_csis_probe(struct platform_device *pdev)
>   mipi_csis_phy_init(state);
>   mipi_csis_phy_reset(state);
>
> - mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - state->regs = devm_ioremap_resource(dev, mem_res);
> + state->regs = devm_platform_ioremap_resource(pdev, 0);
>   if (IS_ERR(state->regs))
>   return PTR_ERR(state->regs);

Re: [PATCH] staging: media: imx: Use devm_platform_ioremap_resource().

[Rui Miguel Silva]

Hi Jeeeun,
On Thu 26 Sep 2019 at 16:55, Jeeeun Evans wrote:
> This patch fixes a warning by coccicheck:
> drivers/staging/media/imx/imx7-mipi-csis.c:973:1-12: WARNING: Use 
> devm_platform_ioremap_resource for state -> regs
>
> Use devm_platform_ioremap_resource helper which wraps platform_get_resource()
> and devm_ioremap_resource() together.
>
> Signed-off-by: Jeeeun Evans 
>

Thanks for the patch.
LGTM.

Reviewed-by: Rui Miguel Silva 

---
Cheers,
Rui


>  drivers/staging/media/imx/imx7-mipi-csis.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c 
> b/drivers/staging/media/imx/imx7-mipi-csis.c
> index 73d8354e618c..bf21db38441f 100644
> --- a/drivers/staging/media/imx/imx7-mipi-csis.c
> +++ b/drivers/staging/media/imx/imx7-mipi-csis.c
> @@ -947,7 +947,6 @@ static void mipi_csis_debugfs_exit(struct csi_state 
> *state)
>  static int mipi_csis_probe(struct platform_device *pdev)
>  {
>   struct device *dev = >dev;
> - struct resource *mem_res;
>   struct csi_state *state;
>   int ret;
>
> @@ -969,8 +968,7 @@ static int mipi_csis_probe(struct platform_device *pdev)
>   mipi_csis_phy_init(state);
>   mipi_csis_phy_reset(state);
>
> - mem_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - state->regs = devm_ioremap_resource(dev, mem_res);
> + state->regs = devm_platform_ioremap_resource(pdev, 0);
>   if (IS_ERR(state->regs))
>   return PTR_ERR(state->regs);

Re: [PATCH] media: imx7-mipi-csis: make array 'registers' static const, makes object smaller

[Rui Miguel Silva]

Hi Colin,
Thanks for the patch.

On Fri 06 Sep 2019 at 16:08, Colin King wrote:
> From: Colin Ian King 
>
> Don't populate the array 'registers' on the stack but instead make it
> static const. Makes the object code smaller by 10 bytes.
>
>
> Before:
>text  data bss dec hex filename
>   20138  5196 128   254626376 
> staging/media/imx/imx7-mipi-csis.o
>
> After:
>text  data bss dec hex filename
>   20032  5292 128   25452636c 
> staging/media/imx/imx7-mipi-csis.o
>
> (gcc version 9.2.1, amd64)
>
> Signed-off-by: Colin Ian King 

Looks very good to me.
Reviewed-by: Rui Miguel Silva 

Cheers,
   Rui
> ---
>  drivers/staging/media/imx/imx7-mipi-csis.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c 
> b/drivers/staging/media/imx/imx7-mipi-csis.c
> index 73d8354e618c..f8a97b7e2535 100644
> --- a/drivers/staging/media/imx/imx7-mipi-csis.c
> +++ b/drivers/staging/media/imx/imx7-mipi-csis.c
> @@ -293,7 +293,7 @@ static int mipi_csis_dump_regs(struct csi_state *state)
>   struct device *dev = >pdev->dev;
>   unsigned int i;
>   u32 cfg;
> - struct {
> + static const struct {
>   u32 offset;
>   const char * const name;
>   } registers[] = {

Re: [PATCH 15/22] media: imx7-media-csi: Create media links in bound notifier

[Rui Miguel Silva]

Hi Steve,
On Tue 06 Aug 2019 at 00:34, Steve Longerbeam wrote:
> Implement a notifier bound op to register media links from the remote
> sub-device's source pad(s) to the CSI sink pad.
>
> Signed-off-by: Steve Longerbeam 
> ---
>  drivers/staging/media/imx/imx7-media-csi.c | 24 ++
>  1 file changed, 24 insertions(+)
>
> diff --git a/drivers/staging/media/imx/imx7-media-csi.c 
> b/drivers/staging/media/imx/imx7-media-csi.c
> index a1c96c52a606..f71ac485f780 100644
> --- a/drivers/staging/media/imx/imx7-media-csi.c
> +++ b/drivers/staging/media/imx/imx7-media-csi.c
> @@ -196,6 +196,11 @@ struct imx7_csi {
>   struct completion last_eof_completion;
>  };
>
> +static inline struct imx7_csi *notifier_to_dev(struct v4l2_async_notifier *n)
>

As the other one add the namespace for the function name:
imx7_csi_notifier_to_dev

other than this, looks good to me.

Cheers,
  Rui
> +{
> + return container_of(n, struct imx7_csi, notifier);
> +}
> +
>  static u32 imx7_csi_reg_read(struct imx7_csi *csi, unsigned int offset)
>  {
>   return readl(csi->regbase + offset);
> @@ -1173,6 +1178,23 @@ static int imx7_csi_parse_endpoint(struct device *dev,
>   return fwnode_device_is_available(asd->match.fwnode) ? 0 : -EINVAL;
>  }
>
> +static int imx7_csi_notify_bound(struct v4l2_async_notifier *notifier,
> +  struct v4l2_subdev *sd,
> +  struct v4l2_async_subdev *asd)
> +{
> + struct imx7_csi *csi = notifier_to_dev(notifier);
> + struct media_pad *sink = >sd.entity.pads[IMX7_CSI_PAD_SINK];
> +
> + return media_create_fwnode_pad_links(sink,
> +  dev_fwnode(csi->sd.dev),
> +  >entity,
> +  dev_fwnode(sd->dev), 0);
> +}
> +
> +static const struct v4l2_async_notifier_operations imx7_csi_notify_ops = {
> + .bound = imx7_csi_notify_bound,
> +};
> +
>  static int imx7_csi_probe(struct platform_device *pdev)
>  {
>   struct device *dev = >dev;
> @@ -1253,6 +1275,8 @@ static int imx7_csi_probe(struct platform_device *pdev)
>
>   v4l2_async_notifier_init(>notifier);
>
> + csi->notifier.ops = _csi_notify_ops;
> +
>   ret = v4l2_async_register_fwnode_subdev(>sd, >notifier,
>   sizeof(struct 
> v4l2_async_subdev),
>   NULL, 0,

Re: [PATCH 14/22] media: imx7-mipi-csis: Create media links in bound notifier

[Rui Miguel Silva]

Hi Steve,
Just to let you know that this series no longer apply clean after
your other series:
media: imx: Fix subdev unregister/register issues

And since you will need to send a new one, some notes...

On Tue 06 Aug 2019 at 00:34, Steve Longerbeam wrote:
> Implement a notifier bound op to register media links from the remote
> sub-device's source pad(s) to the mipi csi-2 receiver sink pad.
>
> Signed-off-by: Steve Longerbeam 
> ---
>  drivers/staging/media/imx/imx7-mipi-csis.c | 25 ++
>  1 file changed, 25 insertions(+)
>
> diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c 
> b/drivers/staging/media/imx/imx7-mipi-csis.c
> index f71d9183cad2..e03b2317a1ac 100644
> --- a/drivers/staging/media/imx/imx7-mipi-csis.c
> +++ b/drivers/staging/media/imx/imx7-mipi-csis.c
> @@ -259,6 +259,12 @@ struct csi_state {
>   bool sink_linked;
>  };
>
> +static inline struct csi_state *
> +notifier_to_csis_state(struct v4l2_async_notifier *n)
>

instead of adding this between structs declaration can you move it
down near the similar converter:

static struct csi_state *mipi_sd_to_csis_state(struct v4l2_subdev *sdev)

and remove the inline since the compiler should do this and add
namespacing function name like the other functions, something like:

static struct csi_state * mipi_notifier_to_csis_state(struct 
v4l2_async_notifier *n)

Just to coherency.

Other than this, looks good to me.

Cheers,
   Rui

> +{
> + return container_of(n, struct csi_state, notifier);
> +}
> +
>  struct csis_pix_format {
>   unsigned int pix_width_alignment;
>   u32 code;
> @@ -863,6 +869,23 @@ static int mipi_csis_parse_endpoint(struct device *dev,
>   return 0;
>  }
>
> +static int mipi_csis_notify_bound(struct v4l2_async_notifier *notifier,
> +   struct v4l2_subdev *sd,
> +   struct v4l2_async_subdev *asd)
> +{
> + struct csi_state *state = notifier_to_csis_state(notifier);
> + struct media_pad *sink = >mipi_sd.entity.pads[CSIS_PAD_SINK];
> +
> + return media_create_fwnode_pad_links(sink,
> +  dev_fwnode(state->mipi_sd.dev),
> +  >entity,
> +  dev_fwnode(sd->dev), 0);
> +}
> +
> +static const struct v4l2_async_notifier_operations mipi_csis_notify_ops = {
> + .bound = mipi_csis_notify_bound,
> +};
> +
>  static int mipi_csis_subdev_init(struct v4l2_subdev *mipi_sd,
>struct platform_device *pdev,
>const struct v4l2_subdev_ops *ops)
> @@ -895,6 +918,8 @@ static int mipi_csis_subdev_init(struct v4l2_subdev 
> *mipi_sd,
>
>   v4l2_async_notifier_init(>notifier);
>
> + state->notifier.ops = _csis_notify_ops;
> +
>   ret = v4l2_async_register_fwnode_subdev(mipi_sd, >notifier,
>   sizeof(struct 
> v4l2_async_subdev),
>   _port, 1,

Re: [PATCH] staging: greybus: light: fix a couple double frees

[Rui Miguel Silva]

Hi Dan,
On Thu 29 Aug 2019 at 13:28, Dan Carpenter wrote:
> The problem is in gb_lights_request_handler().  If we get a request to
> change the config then we release the light with gb_lights_light_release()
> and re-allocated it.  However, if the allocation fails part way through
> then we call gb_lights_light_release() again.  This can lead to a couple
> different double frees where we haven't cleared out the original values:
>
>   gb_lights_light_v4l2_unregister(light);
>   ...
>   kfree(light->channels);
>   kfree(light->name);
>
> I also made a small change to how we set "light->channels_count = 0;".
> The original code handled this part fine and did not cause a use after
> free but it was sort of complicated to read.
>
> Fixes: 2870b52bae4c ("greybus: lights: add lights implementation")
> Signed-off-by: Dan Carpenter 
>

Thanks so much for this, I was looking for some time at this and
was half way to a much less elegant fix then yours.

Acked-by: Rui Miguel Silva 

Cheers,
Rui

> ---
>  drivers/staging/greybus/light.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c
> index 010ae1e9c7fb..40680eaf3974 100644
> --- a/drivers/staging/greybus/light.c
> +++ b/drivers/staging/greybus/light.c
> @@ -1098,21 +1098,21 @@ static void gb_lights_channel_release(struct 
> gb_channel *channel)
>  static void gb_lights_light_release(struct gb_light *light)
>  {
>   int i;
> - int count;
>
>   light->ready = false;
>
> - count = light->channels_count;
> -
>   if (light->has_flash)
>   gb_lights_light_v4l2_unregister(light);
> + light->has_flash = false;
>
> - for (i = 0; i < count; i++) {
> + for (i = 0; i < light->channels_count; i++)
>   gb_lights_channel_release(>channels[i]);
> - light->channels_count--;
> - }
> + light->channels_count = 0;
> +
>   kfree(light->channels);
> + light->channels = NULL;
>   kfree(light->name);
> + light->name = NULL;
>  }
>
>  static void gb_lights_release(struct gb_lights *glights)

Re: [PATCH] media: ov2680: fix a typo in a function name

[Rui Miguel Silva]

Hi Christophe,
Thanks for the patch.

On Sun 21 Jul 2019 at 20:04, Christophe JAILLET wrote:
> All functions in this file starts with 'ov2680_', except ov2860_parse_dt().
>
> This is likely a typo.
>

Yup, weird is to only have one of this. Nice catch.

> rename it to 'ov2680_parse_dt()' (6 and 8)
>
> Signed-off-by: Christophe JAILLET 
>

Reviewed-by: Rui Miguel Silva 

---
Cheers,
Rui

> ---
>  drivers/media/i2c/ov2680.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c
> index b10bcfabaeeb..c59c9e51c380 100644
> --- a/drivers/media/i2c/ov2680.c
> +++ b/drivers/media/i2c/ov2680.c
> @@ -1023,7 +1023,7 @@ static int ov2680_check_id(struct ov2680_dev *sensor)
>   return 0;
>  }
>
> -static int ov2860_parse_dt(struct ov2680_dev *sensor)
> +static int ov2680_parse_dt(struct ov2680_dev *sensor)
>  {
>   struct device *dev = ov2680_to_dev(sensor);
>   int ret;
> @@ -1064,7 +1064,7 @@ static int ov2680_probe(struct i2c_client *client)
>
>   sensor->i2c_client = client;
>
> - ret = ov2860_parse_dt(sensor);
> + ret = ov2680_parse_dt(sensor);
>   if (ret < 0)
>   return -EINVAL;

Re: [PATCH] media: imx7-media-csi: Remove unneeded break after return

[Rui Miguel Silva]

Hi Chinmaya,
Thanks for your patch.

On Sun 30 Jun 2019 at 04:49, Chinmaya Krishnan Mahesh wrote:
> This patch fixes the checkpatch.pl warning:
>
> WARNING: break is not useful after a goto or return

but this is already fixed in the media subsystem tree, by a patch
from Fabio:

964fcacddf media: imx7-media-csi: Remove unneeded break

It is better to use that tree as reference for media fixes,
sometimes some are already fixed there.

Nevertheless many thanks for the patch.

---
Cheers,
Rui


>
> Signed-off-by: Chinmaya Krishnan Mahesh 
> ---
>  drivers/staging/media/imx/imx7-media-csi.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/staging/media/imx/imx7-media-csi.c 
> b/drivers/staging/media/imx/imx7-media-csi.c
> index a708a0340eb1..c15acca1dc0d 100644
> --- a/drivers/staging/media/imx/imx7-media-csi.c
> +++ b/drivers/staging/media/imx/imx7-media-csi.c
> @@ -1021,7 +1021,6 @@ static int imx7_csi_try_fmt(struct imx7_csi *csi,
>   break;
>   default:
>   return -EINVAL;
> - break;
>   }
>   return 0;
>  }

Re: [PATCH v2 2/3] media: imx7-media-csi: add i.MX6UL support

[Rui Miguel Silva]

Hi Sebastien,
On Tue 11 Jun 2019 at 11:03, Sébastien Szymanski wrote:
> On 6/11/19 11:40 AM, Rui Miguel Silva wrote:
>> Hi Sebastien,
>> On Tue 11 Jun 2019 at 09:16, Sébastien Szymanski wrote:
>>> Hi Rui,
>>>
>>> thanks for the review!
>>>
>>> On 6/10/19 12:28 PM, Rui Miguel Silva wrote:
>>>> Hi Sebastien,
>>>> Thanks for the patch.
>>>>
>>>> On Thu 06 Jun 2019 at 16:38, Sébastien Szymanski wrote:
>>>>> i.MX7 and i.MX6UL/L have the same CSI controller. So add i.MX6UL/L support
>>>>> to imx7-media-csi driver.
>>>>>
>>>>> Signed-off-by: Sébastien Szymanski 
>>>>> ---
>>>>>
>>>>> Changes for v2:
>>>>>  - rebase on top of linuxtv/master
>>>>>  - mention i.MX6UL/L in header and Kconfig help text
>>>>>  - rename csi_type to csi_soc_id
>>>>>
>>>>>  drivers/staging/media/imx/Kconfig  |  4 +-
>>>>>  drivers/staging/media/imx/imx7-media-csi.c | 62 --
>>>>>  2 files changed, 49 insertions(+), 17 deletions(-)
>>>>>
>>>>> diff --git a/drivers/staging/media/imx/Kconfig 
>>>>> b/drivers/staging/media/imx/Kconfig
>>>>> index ad3d7df6bb3c..8b6dc42c39e0 100644
>>>>> --- a/drivers/staging/media/imx/Kconfig
>>>>> +++ b/drivers/staging/media/imx/Kconfig
>>>>> @@ -22,11 +22,11 @@ config VIDEO_IMX_CSI
>>>>> A video4linux camera sensor interface driver for i.MX5/6.
>>>>>
>>>>>  config VIDEO_IMX7_CSI
>>>>> - tristate "i.MX7 Camera Sensor Interface driver"
>>>>> + tristate "i.MX6UL/L / i.MX7 Camera Sensor Interface driver"
>>>>>   depends on VIDEO_IMX_MEDIA && VIDEO_DEV && I2C
>>>>>   default y
>>>>>   help
>>>>> Enable support for video4linux camera sensor interface driver for
>>>>> -   i.MX7.
>>>>> +   i.MX6UL/L or i.MX7.
>>>>>  endmenu
>>>>>  endif
>>>>> diff --git a/drivers/staging/media/imx/imx7-media-csi.c 
>>>>> b/drivers/staging/media/imx/imx7-media-csi.c
>>>>> index 9101566f3f67..902bdce594cf 100644
>>>>> --- a/drivers/staging/media/imx/imx7-media-csi.c
>>>>> +++ b/drivers/staging/media/imx/imx7-media-csi.c
>>>>> @@ -1,6 +1,6 @@
>>>>>  // SPDX-License-Identifier: GPL-2.0
>>>>>  /*
>>>>> - * V4L2 Capture CSI Subdev for Freescale i.MX7 SOC
>>>>> + * V4L2 Capture CSI Subdev for Freescale i.MX6UL/L / i.MX7 SOC
>>>>>   *
>>>>>   * Copyright (c) 2019 Linaro Ltd
>>>>>   *
>>>>> @@ -152,6 +152,11 @@
>>>>>  #define CSI_CSICR18  0x48
>>>>>  #define CSI_CSICR19  0x4c
>>>>>
>>>>> +enum csi_soc_id {
>>>>> + IMX7,
>>>>> + IMX6UL
>>>>> +};
>>>>> +
>>>>>  struct imx7_csi {
>>>>>   struct device *dev;
>>>>>   struct v4l2_subdev sd;
>>>>> @@ -191,6 +196,7 @@ struct imx7_csi {
>>>>>   bool is_init;
>>>>>   bool is_streaming;
>>>>>   bool is_csi2;
>>>>> + enum csi_soc_id soc_id;
>>>>>
>>>>>   struct completion last_eof_completion;
>>>>>  };
>>>>> @@ -548,6 +554,14 @@ static int imx7_csi_pad_link_validate(struct 
>>>>> v4l2_subdev *sd,
>>>>>   if (ret)
>>>>>   return ret;
>>>>>
>>>>> + if (csi->soc_id == IMX6UL) {
>>>>> + mutex_lock(>lock);
>>>>> + csi->is_csi2 = false;
>>>>> + mutex_unlock(>lock);
>>>>> +
>>>>> + return 0;
>>>>> + }
>>>>> +
>>>>>   ret = imx7_csi_get_upstream_endpoint(csi, _ep, true);
>>>>>   if (ret) {
>>>>>   v4l2_err(>sd, "failed to find upstream endpoint\n");
>>>>> @@ -757,6 +771,7 @@ static int imx7_csi_configure(struct imx7_csi *csi)
>>>>>   struct v4l2_pix_format *out_pix = >fmt.fmt.pix;
>>>>>   __u32 in_code = csi->format_mbus[IMX7_CSI_PAD_SINK].code;
>>>>>   u32 cr1, cr18;
>>>>> + int width = out_pix->width;
>>

Re: [PATCH v2 2/3] media: imx7-media-csi: add i.MX6UL support

[Rui Miguel Silva]

Hi Sebastien,
On Tue 11 Jun 2019 at 09:16, Sébastien Szymanski wrote:
> Hi Rui,
>
> thanks for the review!
>
> On 6/10/19 12:28 PM, Rui Miguel Silva wrote:
>> Hi Sebastien,
>> Thanks for the patch.
>>
>> On Thu 06 Jun 2019 at 16:38, Sébastien Szymanski wrote:
>>> i.MX7 and i.MX6UL/L have the same CSI controller. So add i.MX6UL/L support
>>> to imx7-media-csi driver.
>>>
>>> Signed-off-by: Sébastien Szymanski 
>>> ---
>>>
>>> Changes for v2:
>>>  - rebase on top of linuxtv/master
>>>  - mention i.MX6UL/L in header and Kconfig help text
>>>  - rename csi_type to csi_soc_id
>>>
>>>  drivers/staging/media/imx/Kconfig  |  4 +-
>>>  drivers/staging/media/imx/imx7-media-csi.c | 62 --
>>>  2 files changed, 49 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/drivers/staging/media/imx/Kconfig 
>>> b/drivers/staging/media/imx/Kconfig
>>> index ad3d7df6bb3c..8b6dc42c39e0 100644
>>> --- a/drivers/staging/media/imx/Kconfig
>>> +++ b/drivers/staging/media/imx/Kconfig
>>> @@ -22,11 +22,11 @@ config VIDEO_IMX_CSI
>>>   A video4linux camera sensor interface driver for i.MX5/6.
>>>
>>>  config VIDEO_IMX7_CSI
>>> -   tristate "i.MX7 Camera Sensor Interface driver"
>>> +   tristate "i.MX6UL/L / i.MX7 Camera Sensor Interface driver"
>>> depends on VIDEO_IMX_MEDIA && VIDEO_DEV && I2C
>>> default y
>>> help
>>>   Enable support for video4linux camera sensor interface driver for
>>> - i.MX7.
>>> + i.MX6UL/L or i.MX7.
>>>  endmenu
>>>  endif
>>> diff --git a/drivers/staging/media/imx/imx7-media-csi.c 
>>> b/drivers/staging/media/imx/imx7-media-csi.c
>>> index 9101566f3f67..902bdce594cf 100644
>>> --- a/drivers/staging/media/imx/imx7-media-csi.c
>>> +++ b/drivers/staging/media/imx/imx7-media-csi.c
>>> @@ -1,6 +1,6 @@
>>>  // SPDX-License-Identifier: GPL-2.0
>>>  /*
>>> - * V4L2 Capture CSI Subdev for Freescale i.MX7 SOC
>>> + * V4L2 Capture CSI Subdev for Freescale i.MX6UL/L / i.MX7 SOC
>>>   *
>>>   * Copyright (c) 2019 Linaro Ltd
>>>   *
>>> @@ -152,6 +152,11 @@
>>>  #define CSI_CSICR180x48
>>>  #define CSI_CSICR190x4c
>>>
>>> +enum csi_soc_id {
>>> +   IMX7,
>>> +   IMX6UL
>>> +};
>>> +
>>>  struct imx7_csi {
>>> struct device *dev;
>>> struct v4l2_subdev sd;
>>> @@ -191,6 +196,7 @@ struct imx7_csi {
>>> bool is_init;
>>> bool is_streaming;
>>> bool is_csi2;
>>> +   enum csi_soc_id soc_id;
>>>
>>> struct completion last_eof_completion;
>>>  };
>>> @@ -548,6 +554,14 @@ static int imx7_csi_pad_link_validate(struct 
>>> v4l2_subdev *sd,
>>> if (ret)
>>> return ret;
>>>
>>> +   if (csi->soc_id == IMX6UL) {
>>> +   mutex_lock(>lock);
>>> +   csi->is_csi2 = false;
>>> +   mutex_unlock(>lock);
>>> +
>>> +   return 0;
>>> +   }
>>> +
>>> ret = imx7_csi_get_upstream_endpoint(csi, _ep, true);
>>> if (ret) {
>>> v4l2_err(>sd, "failed to find upstream endpoint\n");
>>> @@ -757,6 +771,7 @@ static int imx7_csi_configure(struct imx7_csi *csi)
>>> struct v4l2_pix_format *out_pix = >fmt.fmt.pix;
>>> __u32 in_code = csi->format_mbus[IMX7_CSI_PAD_SINK].code;
>>> u32 cr1, cr18;
>>> +   int width = out_pix->width;
>>>
>>> if (out_pix->field == V4L2_FIELD_INTERLACED) {
>>> imx7_csi_deinterlace_enable(csi, true);
>>> @@ -766,15 +781,27 @@ static int imx7_csi_configure(struct imx7_csi *csi)
>>> imx7_csi_buf_stride_set(csi, 0);
>>> }
>>>
>>> -   imx7_csi_set_imagpara(csi, out_pix->width, out_pix->height);
>>> +   cr18 = imx7_csi_reg_read(csi, CSI_CSICR18);
>>> +
>>> +   if (!csi->is_csi2) {
>>> +   if (out_pix->pixelformat == V4L2_PIX_FMT_UYVY ||
>>> +   out_pix->pixelformat == V4L2_PIX_FMT_YUYV)
>>> +   width *= 2;
>>> +
>>> +   imx7_csi_set_imagpara(csi, width, out_pix->height);
>>> +
>>> +   cr18 |= (BIT_B

Re: [PATCH v2 2/3] media: imx7-media-csi: add i.MX6UL support

[Rui Miguel Silva]

Hi Sebastien,
Thanks for the patch.

On Thu 06 Jun 2019 at 16:38, Sébastien Szymanski wrote:
> i.MX7 and i.MX6UL/L have the same CSI controller. So add i.MX6UL/L support
> to imx7-media-csi driver.
>
> Signed-off-by: Sébastien Szymanski 
> ---
>
> Changes for v2:
>  - rebase on top of linuxtv/master
>  - mention i.MX6UL/L in header and Kconfig help text
>  - rename csi_type to csi_soc_id
>
>  drivers/staging/media/imx/Kconfig  |  4 +-
>  drivers/staging/media/imx/imx7-media-csi.c | 62 --
>  2 files changed, 49 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/staging/media/imx/Kconfig 
> b/drivers/staging/media/imx/Kconfig
> index ad3d7df6bb3c..8b6dc42c39e0 100644
> --- a/drivers/staging/media/imx/Kconfig
> +++ b/drivers/staging/media/imx/Kconfig
> @@ -22,11 +22,11 @@ config VIDEO_IMX_CSI
> A video4linux camera sensor interface driver for i.MX5/6.
>
>  config VIDEO_IMX7_CSI
> - tristate "i.MX7 Camera Sensor Interface driver"
> + tristate "i.MX6UL/L / i.MX7 Camera Sensor Interface driver"
>   depends on VIDEO_IMX_MEDIA && VIDEO_DEV && I2C
>   default y
>   help
> Enable support for video4linux camera sensor interface driver for
> -   i.MX7.
> +   i.MX6UL/L or i.MX7.
>  endmenu
>  endif
> diff --git a/drivers/staging/media/imx/imx7-media-csi.c 
> b/drivers/staging/media/imx/imx7-media-csi.c
> index 9101566f3f67..902bdce594cf 100644
> --- a/drivers/staging/media/imx/imx7-media-csi.c
> +++ b/drivers/staging/media/imx/imx7-media-csi.c
> @@ -1,6 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /*
> - * V4L2 Capture CSI Subdev for Freescale i.MX7 SOC
> + * V4L2 Capture CSI Subdev for Freescale i.MX6UL/L / i.MX7 SOC
>   *
>   * Copyright (c) 2019 Linaro Ltd
>   *
> @@ -152,6 +152,11 @@
>  #define CSI_CSICR18  0x48
>  #define CSI_CSICR19  0x4c
>
> +enum csi_soc_id {
> + IMX7,
> + IMX6UL
> +};
> +
>  struct imx7_csi {
>   struct device *dev;
>   struct v4l2_subdev sd;
> @@ -191,6 +196,7 @@ struct imx7_csi {
>   bool is_init;
>   bool is_streaming;
>   bool is_csi2;
> + enum csi_soc_id soc_id;
>
>   struct completion last_eof_completion;
>  };
> @@ -548,6 +554,14 @@ static int imx7_csi_pad_link_validate(struct v4l2_subdev 
> *sd,
>   if (ret)
>   return ret;
>
> + if (csi->soc_id == IMX6UL) {
> + mutex_lock(>lock);
> + csi->is_csi2 = false;
> + mutex_unlock(>lock);
> +
> + return 0;
> + }
> +
>   ret = imx7_csi_get_upstream_endpoint(csi, _ep, true);
>   if (ret) {
>   v4l2_err(>sd, "failed to find upstream endpoint\n");
> @@ -757,6 +771,7 @@ static int imx7_csi_configure(struct imx7_csi *csi)
>   struct v4l2_pix_format *out_pix = >fmt.fmt.pix;
>   __u32 in_code = csi->format_mbus[IMX7_CSI_PAD_SINK].code;
>   u32 cr1, cr18;
> + int width = out_pix->width;
>
>   if (out_pix->field == V4L2_FIELD_INTERLACED) {
>   imx7_csi_deinterlace_enable(csi, true);
> @@ -766,15 +781,27 @@ static int imx7_csi_configure(struct imx7_csi *csi)
>   imx7_csi_buf_stride_set(csi, 0);
>   }
>
> - imx7_csi_set_imagpara(csi, out_pix->width, out_pix->height);
> + cr18 = imx7_csi_reg_read(csi, CSI_CSICR18);
> +
> + if (!csi->is_csi2) {
> + if (out_pix->pixelformat == V4L2_PIX_FMT_UYVY ||
> + out_pix->pixelformat == V4L2_PIX_FMT_YUYV)
> + width *= 2;
> +
> + imx7_csi_set_imagpara(csi, width, out_pix->height);
> +
> + cr18 |= (BIT_BASEADDR_SWITCH_EN | BIT_BASEADDR_SWITCH_SEL |
> + BIT_BASEADDR_CHG_ERR_EN);
> + imx7_csi_reg_write(csi, cr18, CSI_CSICR18);
>
> - if (!csi->is_csi2)
>   return 0;
> + }
> +
> + imx7_csi_set_imagpara(csi, width, out_pix->height);
>
>   cr1 = imx7_csi_reg_read(csi, CSI_CSICR1);
>   cr1 &= ~BIT_GCLK_MODE;
>
> - cr18 = imx7_csi_reg_read(csi, CSI_CSICR18);
>   cr18 &= BIT_MIPI_DATA_FORMAT_MASK;
>   cr18 |= BIT_DATA_FROM_MIPI;
>
> @@ -809,11 +836,9 @@ static void imx7_csi_enable(struct imx7_csi *csi)
>  {
>   imx7_csi_sw_reset(csi);
>
> - if (csi->is_csi2) {
> - imx7_csi_dmareq_rff_enable(csi);
> - imx7_csi_hw_enable_irq(csi);
> - imx7_csi_hw_enable(csi);
> - }
> + imx7_csi_dmareq_rff_enable(csi);
> + imx7_csi_hw_enable_irq(csi);
> + imx7_csi_hw_enable(csi);
>  }
>
>  static void imx7_csi_disable(struct imx7_csi *csi)
> @@ -1166,19 +1191,32 @@ static int imx7_csi_parse_endpoint(struct device *dev,
>   return fwnode_device_is_available(asd->match.fwnode) ? 0 : -EINVAL;
>  }
>
> +static const struct of_device_id imx7_csi_of_match[] = {
> + { .compatible = "fsl,imx7-csi", .data = (void *)IMX7 },
> + { .compatible = "fsl,imx6ul-csi", .data = (void *)IMX6UL },

looking at this again I think we can do this is a different way.
Instead 

Re: [PATCH 1/2] ARM: dts: imx6ul: Add csi node

[Rui Miguel Silva]

Oi Fabio,
On Thu 02 May 2019 at 16:28, Fabio Estevam wrote:
> [Adding Rui]
>
> On Tue, Apr 30, 2019 at 4:47 AM Sébastien Szymanski
>  wrote:
>>
>> Add csi node for i.MX6UL SoC.
>>
>> Signed-off-by: Sébastien Szymanski 
>> ---
>>  arch/arm/boot/dts/imx6ul.dtsi | 11 +++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/imx6ul.dtsi b/arch/arm/boot/dts/imx6ul.dtsi
>> index 62ed30c781ed..af322bc58333 100644
>> --- a/arch/arm/boot/dts/imx6ul.dtsi
>> +++ b/arch/arm/boot/dts/imx6ul.dtsi
>> @@ -951,6 +951,17 @@
>> };
>> };
>>
>> +   csi: csi@21c4000 {
>> +   compatible = "fsl,imx6ul-csi", 
>> "fsl,imx7-csi";
>> +   reg = <0x021c4000 0x4000>;
>> +   interrupts = ;
>> +   clocks = < IMX6UL_CLK_DUMMY>,
>> +< IMX6UL_CLK_CSI>,
>> +< IMX6UL_CLK_DUMMY>;
>> +   clock-names = "axi", "mclk", "dcic";
>
> Also, I understand you followed
> Documentation/devicetree/bindings/media/imx7-csi.txt and passed these
> three clocks, but looking at the i.MX7D and i.MX6UL/ULL Reference
> Manuals, I don't find the  the descriptions for the "axi" and "dcic"
> CSI clocks.
>
> It looks like that only "mclk" is what we really need here.

Yeah, you are right.

>
> Should we change the bindings and the imx7-csi driver to not request
> "axi" and "dcic" clocks?
>
> Rui, what do you think? If you agree I can send a fix for this.

If you please, that would be great. thanks.

---
Cheers,
Rui

Re: [PATCH v2] staging: greybus: power_supply: use struct_size() helper

[Rui Miguel Silva]

Oi Gustavo,
Thanks for the patch, and the rebasing.

On Wed 17 Apr 2019 at 19:44, Gustavo A. R. Silva wrote:
Make use of the struct_size() helper instead of an open-coded 
version
in order to avoid any potential type mistakes, in particular in 
the

context in which this code is being used.

So, replace code of the following form:

sizeof(*resp) + props_count * sizeof(struct 
gb_power_supply_props_desc)


with:

struct_size(resp, props, props_count)

This code was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva 


Reviewed-by: Rui Miguel Silva 

---
Cheers,
Rui



---
Changes in v2:
 - Rebase on top of 47830c1127ef ("staging: greybus: 
 power_supply: fix prop-descriptor request size")


 drivers/staging/greybus/power_supply.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/greybus/power_supply.c 
b/drivers/staging/greybus/power_supply.c

index ae5c0285a942..34b40a409ea3 100644
--- a/drivers/staging/greybus/power_supply.c
+++ b/drivers/staging/greybus/power_supply.c
@@ -520,8 +520,8 @@ static int 
gb_power_supply_prop_descriptors_get(struct gb_power_supply 
*gbpsy)
 
 	op = gb_operation_create(connection,

 GB_POWER_SUPPLY_TYPE_GET_PROP_DESCRIPTORS,
- sizeof(*req), sizeof(*resp) + 
props_count *
- sizeof(struct 
gb_power_supply_props_desc),

+sizeof(*req),
+ struct_size(resp, props, 
props_count),

 GFP_KERNEL);
if (!op)
return -ENOMEM;

Re: [PATCH] staging: greybus: power_supply: Use struct_size() helper

[Rui Miguel Silva]

Hi Gustavo,
Thanks a lot for the patch.

On Wed 03 Apr 2019 at 21:58, Gustavo A. R. Silva wrote:
Make use of the struct_size() helper instead of an open-coded 
version
in order to avoid any potential type mistakes, in particular in 
the

context in which this code is being used.

So, replace code of the following form:

sizeof(*resp) + props_count * sizeof(struct 
gb_power_supply_props_desc)


with:

struct_size(resp, props, props_count)

This code was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva 


What are the odds of 2 people changing same code in greybus in
the same day :).

But it happened, so as Johan asked please rebase on top of his
patch. that would be great.

---
Cheers,
Rui


---
 drivers/staging/greybus/power_supply.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/greybus/power_supply.c 
b/drivers/staging/greybus/power_supply.c

index 0529e5628c24..40cc2d462ba0 100644
--- a/drivers/staging/greybus/power_supply.c
+++ b/drivers/staging/greybus/power_supply.c
@@ -520,8 +520,8 @@ static int 
gb_power_supply_prop_descriptors_get(struct gb_power_supply 
*gbpsy)
 
 	op = gb_operation_create(connection,

 GB_POWER_SUPPLY_TYPE_GET_PROP_DESCRIPTORS,
- sizeof(req), sizeof(*resp) + 
props_count *
- sizeof(struct 
gb_power_supply_props_desc),

+sizeof(req),
+ struct_size(resp, props, 
props_count),

 GFP_KERNEL);
if (!op)
return -ENOMEM;

Re: [PATCH] staging: greybus: power_supply: fix prop-descriptor request size

[Rui Miguel Silva]

Hi Johan,
Thanks for the patch.

On Thu 04 Apr 2019 at 07:53, Johan Hovold wrote:

Since moving the message buffers off the stack, the dynamically
allocated get-prop-descriptor request buffer is incorrectly 
sized due to
using the pointer rather than request-struct size when creating 
the

operation.

Fortunately, the pointer size is always larger than this 
one-byte
request, but this could still cause trouble on the remote end 
due to the

unexpected message size.

Fixes: 9d15134d067e ("greybus: power_supply: rework get 
descriptors")

Cc: stable  # 4.9
Cc: Rui Miguel Silva 
Signed-off-by: Johan Hovold 


Nice catch. LGTM.

Reviewed-by: Rui Miguel Silva 

---
Cheers,
Rui



---
 drivers/staging/greybus/power_supply.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/greybus/power_supply.c 
b/drivers/staging/greybus/power_supply.c

index 0529e5628c24..ae5c0285a942 100644
--- a/drivers/staging/greybus/power_supply.c
+++ b/drivers/staging/greybus/power_supply.c
@@ -520,7 +520,7 @@ static int 
gb_power_supply_prop_descriptors_get(struct gb_power_supply 
*gbpsy)
 
 	op = gb_operation_create(connection,

 GB_POWER_SUPPLY_TYPE_GET_PROP_DESCRIPTORS,
- sizeof(req), sizeof(*resp) + 
props_count *
+ sizeof(*req), sizeof(*resp) + 
props_count *
  sizeof(struct 
 gb_power_supply_props_desc),

 GFP_KERNEL);
if (!op)

Re: [PATCH v5 1/2] media: ov2680: dt: Add bindings for OV2680

[Rui Miguel Silva]

Hi Fabio,
On Tue 24 Apr 2018 at 15:53, Fabio Estevam wrote:

Hi Rui,

On Thu, Apr 19, 2018 at 8:00 AM, Rui Miguel Silva 
<rui.si...@linaro.org> wrote:
Add device tree binding documentation for the OV2680 camera 
sensor.


Reviewed-by: Rob Herring <r...@kernel.org>
CC: devicet...@vger.kernel.org
Signed-off-by: Rui Miguel Silva <rui.si...@linaro.org>
---
 .../devicetree/bindings/media/i2c/ov2680.txt  | 40 
 +++

 1 file changed, 40 insertions(+)
 create mode 100644 
 Documentation/devicetree/bindings/media/i2c/ov2680.txt


diff --git 
a/Documentation/devicetree/bindings/media/i2c/ov2680.txt 
b/Documentation/devicetree/bindings/media/i2c/ov2680.txt

new file mode 100644
index ..0e29f1a113c0
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
@@ -0,0 +1,40 @@
+* Omnivision OV2680 MIPI CSI-2 sensor
+
+Required Properties:
+- compatible: should be "ovti,ov2680".
+- clocks: reference to the xvclk input clock.
+- clock-names: should be "xvclk".


You missed to pass the camera power supplies as required 
properties:


Urgh, yes, you are right, I will add this.

---
Cheers,
Rui



DOVDD-supply
AVDD-supply
DVDD-supply

Re: [PATCH v5 1/2] media: ov2680: dt: Add bindings for OV2680

[Rui Miguel Silva]

Hi Fabio,
On Tue 24 Apr 2018 at 15:53, Fabio Estevam wrote:

Hi Rui,

On Thu, Apr 19, 2018 at 8:00 AM, Rui Miguel Silva 
 wrote:
Add device tree binding documentation for the OV2680 camera 
sensor.


Reviewed-by: Rob Herring 
CC: devicet...@vger.kernel.org
Signed-off-by: Rui Miguel Silva 
---
 .../devicetree/bindings/media/i2c/ov2680.txt  | 40 
 +++

 1 file changed, 40 insertions(+)
 create mode 100644 
 Documentation/devicetree/bindings/media/i2c/ov2680.txt


diff --git 
a/Documentation/devicetree/bindings/media/i2c/ov2680.txt 
b/Documentation/devicetree/bindings/media/i2c/ov2680.txt

new file mode 100644
index ..0e29f1a113c0
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
@@ -0,0 +1,40 @@
+* Omnivision OV2680 MIPI CSI-2 sensor
+
+Required Properties:
+- compatible: should be "ovti,ov2680".
+- clocks: reference to the xvclk input clock.
+- clock-names: should be "xvclk".


You missed to pass the camera power supplies as required 
properties:


Urgh, yes, you are right, I will add this.

---
Cheers,
Rui



DOVDD-supply
AVDD-supply
DVDD-supply

Re: [PATCH v5 1/2] media: ov2680: dt: Add bindings for OV2680

[Rui Miguel Silva]

Hi Fabio,
Thanks for the review.

On Mon 23 Apr 2018 at 14:11, Fabio Estevam wrote:

Hi Rui,

On Thu, Apr 19, 2018 at 8:00 AM, Rui Miguel Silva 
<rui.si...@linaro.org> wrote:



+Optional Properties:
+- powerdown-gpios: reference to the GPIO connected to the 
powerdown pin,
+if any. This is an active high signal to 
the OV2680.


I looked at the OV2680 datasheet and I see a pin called XSHUTDN, 
which has

the following description:

XSHUTDN: reset and power down (active low with internal pull 
down resistor)


So it should be active low, not active high.


Yes, you are correct, I will fix this, and the dts entry.

Thanks.

---
Cheers,
Rui

Re: [PATCH v5 1/2] media: ov2680: dt: Add bindings for OV2680

[Rui Miguel Silva]

Hi Fabio,
Thanks for the review.

On Mon 23 Apr 2018 at 14:11, Fabio Estevam wrote:

Hi Rui,

On Thu, Apr 19, 2018 at 8:00 AM, Rui Miguel Silva 
 wrote:



+Optional Properties:
+- powerdown-gpios: reference to the GPIO connected to the 
powerdown pin,
+if any. This is an active high signal to 
the OV2680.


I looked at the OV2680 datasheet and I see a pin called XSHUTDN, 
which has

the following description:

XSHUTDN: reset and power down (active low with internal pull 
down resistor)


So it should be active low, not active high.


Yes, you are correct, I will fix this, and the dts entry.

Thanks.

---
Cheers,
Rui

[PATCH v5 2/2] media: ov2680: Add Omnivision OV2680 sensor driver

[Rui Miguel Silva]

This patch adds V4L2 sub-device driver for OV2680 image sensor.
The OV2680 is a 1/5" CMOS color sensor from Omnivision.
Supports output format: 10-bit Raw RGB.
The OV2680 has a single lane MIPI interface.

The driver exposes following V4L2 controls:
- auto/manual exposure,
- exposure,
- auto/manual gain,
- gain,
- horizontal/vertical flip,
- test pattern menu.
Supported resolution are only: QUXGA, 720P, UXGA.

Signed-off-by: Rui Miguel Silva <rui.si...@linaro.org>
---
 drivers/media/i2c/Kconfig  |   12 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov2680.c | 1134 
 3 files changed, 1147 insertions(+)
 create mode 100644 drivers/media/i2c/ov2680.c

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 541f0d28afd8..3a815825ad1b 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -606,6 +606,18 @@ config VIDEO_OV2659
  To compile this driver as a module, choose M here: the
  module will be called ov2659.
 
+config VIDEO_OV2680
+   tristate "OmniVision OV2680 sensor support"
+   depends on VIDEO_V4L2 && I2C && MEDIA_CONTROLLER
+   depends on MEDIA_CAMERA_SUPPORT
+   select V4L2_FWNODE
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the OmniVision
+ OV2680 camera.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ov2680.
+
 config VIDEO_OV2685
tristate "OmniVision OV2685 sensor support"
depends on VIDEO_V4L2 && I2C && MEDIA_CONTROLLER
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index ea34aee1a85a..9539c0855e63 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -63,6 +63,7 @@ obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o
 obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
 obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
 obj-$(CONFIG_VIDEO_OV2640) += ov2640.o
+obj-$(CONFIG_VIDEO_OV2680) += ov2680.o
 obj-$(CONFIG_VIDEO_OV2685) += ov2685.o
 obj-$(CONFIG_VIDEO_OV5640) += ov5640.o
 obj-$(CONFIG_VIDEO_OV5645) += ov5645.o
diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c
new file mode 100644
index ..cc3d06096d51
--- /dev/null
+++ b/drivers/media/i2c/ov2680.c
@@ -0,0 +1,1134 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Omnivision OV2680 CMOS Image Sensor driver
+ *
+ * Copyright (C) 2018 Linaro Ltd
+ *
+ * Based on OV5640 Sensor Driver
+ * Copyright (C) 2011-2013 Freescale Semiconductor, Inc. All Rights Reserved.
+ * Copyright (C) 2014-2017 Mentor Graphics Inc.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#define OV2680_XVCLK_VALUE 2400
+
+#define OV2680_CHIP_ID 0x2680
+
+#define OV2680_REG_STREAM_CTRL 0x0100
+#define OV2680_REG_SOFT_RESET  0x0103
+
+#define OV2680_REG_CHIP_ID_HIGH0x300a
+#define OV2680_REG_CHIP_ID_LOW 0x300b
+
+#define OV2680_REG_R_MANUAL0x3503
+#define OV2680_REG_GAIN_PK 0x350a
+#define OV2680_REG_EXPOSURE_PK_HIGH0x3500
+#define OV2680_REG_TIMING_HTS  0x380c
+#define OV2680_REG_TIMING_VTS  0x380e
+#define OV2680_REG_FORMAT1 0x3820
+#define OV2680_REG_FORMAT2 0x3821
+
+#define OV2680_REG_ISP_CTRL00  0x5080
+
+#define OV2680_FRAME_RATE  30
+
+#define OV2680_REG_VALUE_8BIT  1
+#define OV2680_REG_VALUE_16BIT 2
+#define OV2680_REG_VALUE_24BIT 3
+
+#define OV2680_WIDTH_MAX   1600
+#define OV2680_HEIGHT_MAX  1200
+
+enum ov2680_mode_id {
+   OV2680_MODE_QUXGA_800_600,
+   OV2680_MODE_720P_1280_720,
+   OV2680_MODE_UXGA_1600_1200,
+   OV2680_MODE_MAX,
+};
+
+struct reg_value {
+   u16 reg_addr;
+   u8 val;
+};
+
+struct ov2680_mode_info {
+   const char *name;
+   enum ov2680_mode_id id;
+   u32 width;
+   u32 height;
+   const struct reg_value *reg_data;
+   u32 reg_data_size;
+};
+
+struct ov2680_ctrls {
+   struct v4l2_ctrl_handler handler;
+   struct {
+   struct v4l2_ctrl *auto_exp;
+   struct v4l2_ctrl *exposure;
+   };
+   struct {
+   struct v4l2_ctrl *auto_gain;
+   struct v4l2_ctrl *gain;
+   };
+
+   struct v4l2_ctrl *hflip;
+   struct v4l2_ctrl *vflip;
+   struct v4l2_ctrl *test_pattern;
+};
+
+struct ov2680_dev {
+   struct i2c_client   *i2c_client;
+   struct v4l2_subdev  sd;
+
+   struct media_padpad;
+   struct clk  *xvclk;
+   u32 xvclk_freq;
+
+   struct gpio_desc*pwdn_gpio;
+   struct mutexlock; /* protect members */
+
+   boolmode_pend

[PATCH v5 2/2] media: ov2680: Add Omnivision OV2680 sensor driver

[Rui Miguel Silva]

This patch adds V4L2 sub-device driver for OV2680 image sensor.
The OV2680 is a 1/5" CMOS color sensor from Omnivision.
Supports output format: 10-bit Raw RGB.
The OV2680 has a single lane MIPI interface.

The driver exposes following V4L2 controls:
- auto/manual exposure,
- exposure,
- auto/manual gain,
- gain,
- horizontal/vertical flip,
- test pattern menu.
Supported resolution are only: QUXGA, 720P, UXGA.

Signed-off-by: Rui Miguel Silva 
---
 drivers/media/i2c/Kconfig  |   12 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov2680.c | 1134 
 3 files changed, 1147 insertions(+)
 create mode 100644 drivers/media/i2c/ov2680.c

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 541f0d28afd8..3a815825ad1b 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -606,6 +606,18 @@ config VIDEO_OV2659
  To compile this driver as a module, choose M here: the
  module will be called ov2659.
 
+config VIDEO_OV2680
+   tristate "OmniVision OV2680 sensor support"
+   depends on VIDEO_V4L2 && I2C && MEDIA_CONTROLLER
+   depends on MEDIA_CAMERA_SUPPORT
+   select V4L2_FWNODE
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the OmniVision
+ OV2680 camera.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ov2680.
+
 config VIDEO_OV2685
tristate "OmniVision OV2685 sensor support"
depends on VIDEO_V4L2 && I2C && MEDIA_CONTROLLER
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index ea34aee1a85a..9539c0855e63 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -63,6 +63,7 @@ obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o
 obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
 obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
 obj-$(CONFIG_VIDEO_OV2640) += ov2640.o
+obj-$(CONFIG_VIDEO_OV2680) += ov2680.o
 obj-$(CONFIG_VIDEO_OV2685) += ov2685.o
 obj-$(CONFIG_VIDEO_OV5640) += ov5640.o
 obj-$(CONFIG_VIDEO_OV5645) += ov5645.o
diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c
new file mode 100644
index ..cc3d06096d51
--- /dev/null
+++ b/drivers/media/i2c/ov2680.c
@@ -0,0 +1,1134 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Omnivision OV2680 CMOS Image Sensor driver
+ *
+ * Copyright (C) 2018 Linaro Ltd
+ *
+ * Based on OV5640 Sensor Driver
+ * Copyright (C) 2011-2013 Freescale Semiconductor, Inc. All Rights Reserved.
+ * Copyright (C) 2014-2017 Mentor Graphics Inc.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#define OV2680_XVCLK_VALUE 2400
+
+#define OV2680_CHIP_ID 0x2680
+
+#define OV2680_REG_STREAM_CTRL 0x0100
+#define OV2680_REG_SOFT_RESET  0x0103
+
+#define OV2680_REG_CHIP_ID_HIGH0x300a
+#define OV2680_REG_CHIP_ID_LOW 0x300b
+
+#define OV2680_REG_R_MANUAL0x3503
+#define OV2680_REG_GAIN_PK 0x350a
+#define OV2680_REG_EXPOSURE_PK_HIGH0x3500
+#define OV2680_REG_TIMING_HTS  0x380c
+#define OV2680_REG_TIMING_VTS  0x380e
+#define OV2680_REG_FORMAT1 0x3820
+#define OV2680_REG_FORMAT2 0x3821
+
+#define OV2680_REG_ISP_CTRL00  0x5080
+
+#define OV2680_FRAME_RATE  30
+
+#define OV2680_REG_VALUE_8BIT  1
+#define OV2680_REG_VALUE_16BIT 2
+#define OV2680_REG_VALUE_24BIT 3
+
+#define OV2680_WIDTH_MAX   1600
+#define OV2680_HEIGHT_MAX  1200
+
+enum ov2680_mode_id {
+   OV2680_MODE_QUXGA_800_600,
+   OV2680_MODE_720P_1280_720,
+   OV2680_MODE_UXGA_1600_1200,
+   OV2680_MODE_MAX,
+};
+
+struct reg_value {
+   u16 reg_addr;
+   u8 val;
+};
+
+struct ov2680_mode_info {
+   const char *name;
+   enum ov2680_mode_id id;
+   u32 width;
+   u32 height;
+   const struct reg_value *reg_data;
+   u32 reg_data_size;
+};
+
+struct ov2680_ctrls {
+   struct v4l2_ctrl_handler handler;
+   struct {
+   struct v4l2_ctrl *auto_exp;
+   struct v4l2_ctrl *exposure;
+   };
+   struct {
+   struct v4l2_ctrl *auto_gain;
+   struct v4l2_ctrl *gain;
+   };
+
+   struct v4l2_ctrl *hflip;
+   struct v4l2_ctrl *vflip;
+   struct v4l2_ctrl *test_pattern;
+};
+
+struct ov2680_dev {
+   struct i2c_client   *i2c_client;
+   struct v4l2_subdev  sd;
+
+   struct media_padpad;
+   struct clk  *xvclk;
+   u32 xvclk_freq;
+
+   struct gpio_desc*pwdn_gpio;
+   struct mutexlock; /* protect members */
+
+   boolmode_pending_changes;
+  

[PATCH v5 1/2] media: ov2680: dt: Add bindings for OV2680

[Rui Miguel Silva]

Add device tree binding documentation for the OV2680 camera sensor.

Reviewed-by: Rob Herring <r...@kernel.org>
CC: devicet...@vger.kernel.org
Signed-off-by: Rui Miguel Silva <rui.si...@linaro.org>
---
 .../devicetree/bindings/media/i2c/ov2680.txt  | 40 +++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/ov2680.txt 
b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
new file mode 100644
index ..0e29f1a113c0
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
@@ -0,0 +1,40 @@
+* Omnivision OV2680 MIPI CSI-2 sensor
+
+Required Properties:
+- compatible: should be "ovti,ov2680".
+- clocks: reference to the xvclk input clock.
+- clock-names: should be "xvclk".
+
+Optional Properties:
+- powerdown-gpios: reference to the GPIO connected to the powerdown pin,
+if any. This is an active high signal to the OV2680.
+
+The device node must contain one 'port' child node for its digital output
+video port, and this port must have a single endpoint in accordance with
+ the video interface bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Endpoint node required properties for CSI-2 connection are:
+- remote-endpoint: a phandle to the bus receiver's endpoint node.
+- clock-lanes: should be set to <0> (clock lane on hardware lane 0).
+- data-lanes: should be set to <1> (one CSI-2 lane supported).
+ 
+Example:
+
+ {
+   ov2680: camera-sensor@36 {
+   compatible = "ovti,ov2680";
+   reg = <0x36>;
+   clocks = <>;
+   clock-names = "xvclk";
+   powerdown-gpios = < 3 GPIO_ACTIVE_HIGH>;
+
+   port {
+   ov2680_mipi_ep: endpoint {
+   remote-endpoint = <_sensor_ep>;
+   clock-lanes = <0>;
+   data-lanes = <1>;
+   };
+   };
+   };
+};
-- 
2.17.0

[PATCH v5 1/2] media: ov2680: dt: Add bindings for OV2680

[Rui Miguel Silva]

Add device tree binding documentation for the OV2680 camera sensor.

Reviewed-by: Rob Herring 
CC: devicet...@vger.kernel.org
Signed-off-by: Rui Miguel Silva 
---
 .../devicetree/bindings/media/i2c/ov2680.txt  | 40 +++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/ov2680.txt 
b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
new file mode 100644
index ..0e29f1a113c0
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
@@ -0,0 +1,40 @@
+* Omnivision OV2680 MIPI CSI-2 sensor
+
+Required Properties:
+- compatible: should be "ovti,ov2680".
+- clocks: reference to the xvclk input clock.
+- clock-names: should be "xvclk".
+
+Optional Properties:
+- powerdown-gpios: reference to the GPIO connected to the powerdown pin,
+if any. This is an active high signal to the OV2680.
+
+The device node must contain one 'port' child node for its digital output
+video port, and this port must have a single endpoint in accordance with
+ the video interface bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Endpoint node required properties for CSI-2 connection are:
+- remote-endpoint: a phandle to the bus receiver's endpoint node.
+- clock-lanes: should be set to <0> (clock lane on hardware lane 0).
+- data-lanes: should be set to <1> (one CSI-2 lane supported).
+ 
+Example:
+
+ {
+   ov2680: camera-sensor@36 {
+   compatible = "ovti,ov2680";
+   reg = <0x36>;
+   clocks = <>;
+   clock-names = "xvclk";
+   powerdown-gpios = < 3 GPIO_ACTIVE_HIGH>;
+
+   port {
+   ov2680_mipi_ep: endpoint {
+   remote-endpoint = <_sensor_ep>;
+   clock-lanes = <0>;
+   data-lanes = <1>;
+   };
+   };
+   };
+};
-- 
2.17.0

[PATCH v5 0/2] media: Introduce Omnivision OV2680 driver

[Rui Miguel Silva]

Add driver and bindings for the OV2680 2 megapixel CMOS 1/5" sensor, which has
a single MIPI lane interface and output format of 10-bit Raw RGB.

Features supported are described in PATCH 2/2.

v4->v5:
Fixes for v4l2-compliance tests:
- add init_cfg
- add some input arguments validations
- fix format_try set

v3->v4:
Sakari Ailus:
   - remove auto_{exposure|gain}_enable and direct call the set functions
   - add separe control sets to gain and exposure
   - fix number of controls allocated
   - check the exact frequency that it is supported

v2->v3:
Rob Herring:
- add Reviewed-by tag to dts PATCH 1/1

Sakari Ailus:
- align register values with bracket
- redone the {write|read}_reg i2c functions
- add bayer order handling with flip and mirror controls
- fix error path in probe release resources
- remove i2c_device_id and use probe_new

Myself:
- remove ; at the end of macros

v1->v2:
Fabio Estevam:
- s/OV5640/OV2680 in PATCH 1/2 changelog

Sakari Ailus:
- add description on endpoint properties in bindings
- add single endpoint in bindings
- drop OF dependency
- cleanup includes
- fix case in Color Bars
- remove frame rate selection
- 8/16/24 bit register access in the same transaction
- merge _reset and _soft_reset to _enable and rename it to power_on
- _gain_set use only the gain value (drop & 0x7ff)
- _gain_get remove the (0x377)
- single write/read at _exposure_set/get use write_reg24/read_reg24
- move mode_set_direct to _mode_set
- _mode_set set auto exposure/gain based on ctrl value
- s_frame_interval equal to g_frame_interval
- use closest match from: v4l: common: Add a function to obtain best size 
from a list
- check v4l2_ctrl_new_std return in _init

- fix gain manual value in auto_cluster

Cheers,
Rui


Rui Miguel Silva (2):
  media: ov2680: dt: Add bindings for OV2680
  media: ov2680: Add Omnivision OV2680 sensor driver

 .../devicetree/bindings/media/i2c/ov2680.txt  |   40 +
 drivers/media/i2c/Kconfig |   12 +
 drivers/media/i2c/Makefile|1 +
 drivers/media/i2c/ov2680.c| 1134 +
 4 files changed, 1187 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt
 create mode 100644 drivers/media/i2c/ov2680.c

-- 
2.17.0

[PATCH v5 0/2] media: Introduce Omnivision OV2680 driver

[Rui Miguel Silva]

Add driver and bindings for the OV2680 2 megapixel CMOS 1/5" sensor, which has
a single MIPI lane interface and output format of 10-bit Raw RGB.

Features supported are described in PATCH 2/2.

v4->v5:
Fixes for v4l2-compliance tests:
- add init_cfg
- add some input arguments validations
- fix format_try set

v3->v4:
Sakari Ailus:
   - remove auto_{exposure|gain}_enable and direct call the set functions
   - add separe control sets to gain and exposure
   - fix number of controls allocated
   - check the exact frequency that it is supported

v2->v3:
Rob Herring:
- add Reviewed-by tag to dts PATCH 1/1

Sakari Ailus:
- align register values with bracket
- redone the {write|read}_reg i2c functions
- add bayer order handling with flip and mirror controls
- fix error path in probe release resources
- remove i2c_device_id and use probe_new

Myself:
- remove ; at the end of macros

v1->v2:
Fabio Estevam:
- s/OV5640/OV2680 in PATCH 1/2 changelog

Sakari Ailus:
- add description on endpoint properties in bindings
- add single endpoint in bindings
- drop OF dependency
- cleanup includes
- fix case in Color Bars
- remove frame rate selection
- 8/16/24 bit register access in the same transaction
- merge _reset and _soft_reset to _enable and rename it to power_on
- _gain_set use only the gain value (drop & 0x7ff)
- _gain_get remove the (0x377)
- single write/read at _exposure_set/get use write_reg24/read_reg24
- move mode_set_direct to _mode_set
- _mode_set set auto exposure/gain based on ctrl value
- s_frame_interval equal to g_frame_interval
- use closest match from: v4l: common: Add a function to obtain best size 
from a list
- check v4l2_ctrl_new_std return in _init

- fix gain manual value in auto_cluster

Cheers,
Rui


Rui Miguel Silva (2):
  media: ov2680: dt: Add bindings for OV2680
  media: ov2680: Add Omnivision OV2680 sensor driver

 .../devicetree/bindings/media/i2c/ov2680.txt  |   40 +
 drivers/media/i2c/Kconfig |   12 +
 drivers/media/i2c/Makefile|1 +
 drivers/media/i2c/ov2680.c| 1134 +
 4 files changed, 1187 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt
 create mode 100644 drivers/media/i2c/ov2680.c

-- 
2.17.0

[PATCH v4 1/2] media: ov2680: dt: Add bindings for OV2680

[Rui Miguel Silva]

Add device tree binding documentation for the OV2680 camera sensor.

Reviewed-by: Rob Herring <r...@kernel.org>
CC: devicet...@vger.kernel.org
Signed-off-by: Rui Miguel Silva <rui.si...@linaro.org>
---
 .../devicetree/bindings/media/i2c/ov2680.txt   | 40 ++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/ov2680.txt 
b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
new file mode 100644
index ..0e29f1a113c0
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
@@ -0,0 +1,40 @@
+* Omnivision OV2680 MIPI CSI-2 sensor
+
+Required Properties:
+- compatible: should be "ovti,ov2680".
+- clocks: reference to the xvclk input clock.
+- clock-names: should be "xvclk".
+
+Optional Properties:
+- powerdown-gpios: reference to the GPIO connected to the powerdown pin,
+if any. This is an active high signal to the OV2680.
+
+The device node must contain one 'port' child node for its digital output
+video port, and this port must have a single endpoint in accordance with
+ the video interface bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Endpoint node required properties for CSI-2 connection are:
+- remote-endpoint: a phandle to the bus receiver's endpoint node.
+- clock-lanes: should be set to <0> (clock lane on hardware lane 0).
+- data-lanes: should be set to <1> (one CSI-2 lane supported).
+ 
+Example:
+
+ {
+   ov2680: camera-sensor@36 {
+   compatible = "ovti,ov2680";
+   reg = <0x36>;
+   clocks = <>;
+   clock-names = "xvclk";
+   powerdown-gpios = < 3 GPIO_ACTIVE_HIGH>;
+
+   port {
+   ov2680_mipi_ep: endpoint {
+   remote-endpoint = <_sensor_ep>;
+   clock-lanes = <0>;
+   data-lanes = <1>;
+   };
+   };
+   };
+};
-- 
2.16.3

[PATCH v4 1/2] media: ov2680: dt: Add bindings for OV2680

[Rui Miguel Silva]

Add device tree binding documentation for the OV2680 camera sensor.

Reviewed-by: Rob Herring 
CC: devicet...@vger.kernel.org
Signed-off-by: Rui Miguel Silva 
---
 .../devicetree/bindings/media/i2c/ov2680.txt   | 40 ++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/ov2680.txt 
b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
new file mode 100644
index ..0e29f1a113c0
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
@@ -0,0 +1,40 @@
+* Omnivision OV2680 MIPI CSI-2 sensor
+
+Required Properties:
+- compatible: should be "ovti,ov2680".
+- clocks: reference to the xvclk input clock.
+- clock-names: should be "xvclk".
+
+Optional Properties:
+- powerdown-gpios: reference to the GPIO connected to the powerdown pin,
+if any. This is an active high signal to the OV2680.
+
+The device node must contain one 'port' child node for its digital output
+video port, and this port must have a single endpoint in accordance with
+ the video interface bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Endpoint node required properties for CSI-2 connection are:
+- remote-endpoint: a phandle to the bus receiver's endpoint node.
+- clock-lanes: should be set to <0> (clock lane on hardware lane 0).
+- data-lanes: should be set to <1> (one CSI-2 lane supported).
+ 
+Example:
+
+ {
+   ov2680: camera-sensor@36 {
+   compatible = "ovti,ov2680";
+   reg = <0x36>;
+   clocks = <>;
+   clock-names = "xvclk";
+   powerdown-gpios = < 3 GPIO_ACTIVE_HIGH>;
+
+   port {
+   ov2680_mipi_ep: endpoint {
+   remote-endpoint = <_sensor_ep>;
+   clock-lanes = <0>;
+   data-lanes = <1>;
+   };
+   };
+   };
+};
-- 
2.16.3

[PATCH v4 2/2] media: ov2680: Add Omnivision OV2680 sensor driver

[Rui Miguel Silva]

This patch adds V4L2 sub-device driver for OV2680 image sensor.
The OV2680 is a 1/5" CMOS color sensor from Omnivision.
Supports output format: 10-bit Raw RGB.
The OV2680 has a single lane MIPI interface.

The driver exposes following V4L2 controls:
- auto/manual exposure,
- exposure,
- auto/manual gain,
- gain,
- horizontal/vertical flip,
- test pattern menu.
Supported resolution are only: QUXGA, 720P, UXGA.

Signed-off-by: Rui Miguel Silva <rui.si...@linaro.org>
---
 drivers/media/i2c/Kconfig  |   12 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov2680.c |  
 3 files changed, 1124 insertions(+)
 create mode 100644 drivers/media/i2c/ov2680.c

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 9f18cd296841..39dc9f236ffa 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -586,6 +586,18 @@ config VIDEO_OV2659
  To compile this driver as a module, choose M here: the
  module will be called ov2659.
 
+config VIDEO_OV2680
+   tristate "OmniVision OV2680 sensor support"
+   depends on GPIOLIB && VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API
+   depends on MEDIA_CAMERA_SUPPORT
+   select V4L2_FWNODE
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the OmniVision
+ OV2680 camera sensor with a MIPI CSI-2 interface.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ov2680.
+
 config VIDEO_OV5640
tristate "OmniVision OV5640 sensor support"
depends on OF
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index c0f94cd8d56d..d0aba4d37b8d 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o
 obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
 obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
 obj-$(CONFIG_VIDEO_OV2640) += ov2640.o
+obj-$(CONFIG_VIDEO_OV2680) += ov2680.o
 obj-$(CONFIG_VIDEO_OV5640) += ov5640.o
 obj-$(CONFIG_VIDEO_OV5645) += ov5645.o
 obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c
new file mode 100644
index ..1af6175e09d5
--- /dev/null
+++ b/drivers/media/i2c/ov2680.c
@@ -0,0 +1, @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Omnivision OV2680 CMOS Image Sensor driver
+ *
+ * Copyright (C) 2018 Linaro Ltd
+ *
+ * Based on OV5640 Sensor Driver
+ * Copyright (C) 2011-2013 Freescale Semiconductor, Inc. All Rights Reserved.
+ * Copyright (C) 2014-2017 Mentor Graphics Inc.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#define OV2680_XVCLK_VALUE 2400
+
+#define OV2680_CHIP_ID 0x2680
+
+#define OV2680_REG_STREAM_CTRL 0x0100
+#define OV2680_REG_SOFT_RESET  0x0103
+
+#define OV2680_REG_CHIP_ID_HIGH0x300a
+#define OV2680_REG_CHIP_ID_LOW 0x300b
+
+#define OV2680_REG_R_MANUAL0x3503
+#define OV2680_REG_GAIN_PK 0x350a
+#define OV2680_REG_EXPOSURE_PK_HIGH0x3500
+#define OV2680_REG_TIMING_HTS  0x380c
+#define OV2680_REG_TIMING_VTS  0x380e
+#define OV2680_REG_FORMAT1 0x3820
+#define OV2680_REG_FORMAT2 0x3821
+
+#define OV2680_REG_ISP_CTRL00  0x5080
+
+#define OV2680_FRAME_RATE  30
+
+#define OV2680_REG_VALUE_8BIT  1
+#define OV2680_REG_VALUE_16BIT 2
+#define OV2680_REG_VALUE_24BIT 3
+
+enum ov2680_mode_id {
+   OV2680_MODE_QUXGA_800_600,
+   OV2680_MODE_720P_1280_720,
+   OV2680_MODE_UXGA_1600_1200,
+   OV2680_MODE_MAX,
+};
+
+struct reg_value {
+   u16 reg_addr;
+   u8 val;
+};
+
+struct ov2680_mode_info {
+   const char *name;
+   enum ov2680_mode_id id;
+   u32 width;
+   u32 height;
+   const struct reg_value *reg_data;
+   u32 reg_data_size;
+};
+
+struct ov2680_ctrls {
+   struct v4l2_ctrl_handler handler;
+   struct {
+   struct v4l2_ctrl *auto_exp;
+   struct v4l2_ctrl *exposure;
+   };
+   struct {
+   struct v4l2_ctrl *auto_gain;
+   struct v4l2_ctrl *gain;
+   };
+
+   struct v4l2_ctrl *hflip;
+   struct v4l2_ctrl *vflip;
+   struct v4l2_ctrl *test_pattern;
+};
+
+struct ov2680_dev {
+   struct i2c_client   *i2c_client;
+   struct v4l2_subdev  sd;
+
+   struct media_padpad;
+   struct clk  *xvclk;
+   u32 xvclk_freq;
+
+   struct gpio_desc*pwdn_gpio;
+   struct mutexlock; /* protect members */
+
+   boolmode_pending_changes;
+   boolis_enable

[PATCH v4 2/2] media: ov2680: Add Omnivision OV2680 sensor driver

[Rui Miguel Silva]

This patch adds V4L2 sub-device driver for OV2680 image sensor.
The OV2680 is a 1/5" CMOS color sensor from Omnivision.
Supports output format: 10-bit Raw RGB.
The OV2680 has a single lane MIPI interface.

The driver exposes following V4L2 controls:
- auto/manual exposure,
- exposure,
- auto/manual gain,
- gain,
- horizontal/vertical flip,
- test pattern menu.
Supported resolution are only: QUXGA, 720P, UXGA.

Signed-off-by: Rui Miguel Silva 
---
 drivers/media/i2c/Kconfig  |   12 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov2680.c |  
 3 files changed, 1124 insertions(+)
 create mode 100644 drivers/media/i2c/ov2680.c

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 9f18cd296841..39dc9f236ffa 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -586,6 +586,18 @@ config VIDEO_OV2659
  To compile this driver as a module, choose M here: the
  module will be called ov2659.
 
+config VIDEO_OV2680
+   tristate "OmniVision OV2680 sensor support"
+   depends on GPIOLIB && VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API
+   depends on MEDIA_CAMERA_SUPPORT
+   select V4L2_FWNODE
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the OmniVision
+ OV2680 camera sensor with a MIPI CSI-2 interface.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ov2680.
+
 config VIDEO_OV5640
tristate "OmniVision OV5640 sensor support"
depends on OF
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index c0f94cd8d56d..d0aba4d37b8d 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o
 obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
 obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
 obj-$(CONFIG_VIDEO_OV2640) += ov2640.o
+obj-$(CONFIG_VIDEO_OV2680) += ov2680.o
 obj-$(CONFIG_VIDEO_OV5640) += ov5640.o
 obj-$(CONFIG_VIDEO_OV5645) += ov5645.o
 obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c
new file mode 100644
index ..1af6175e09d5
--- /dev/null
+++ b/drivers/media/i2c/ov2680.c
@@ -0,0 +1, @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Omnivision OV2680 CMOS Image Sensor driver
+ *
+ * Copyright (C) 2018 Linaro Ltd
+ *
+ * Based on OV5640 Sensor Driver
+ * Copyright (C) 2011-2013 Freescale Semiconductor, Inc. All Rights Reserved.
+ * Copyright (C) 2014-2017 Mentor Graphics Inc.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#define OV2680_XVCLK_VALUE 2400
+
+#define OV2680_CHIP_ID 0x2680
+
+#define OV2680_REG_STREAM_CTRL 0x0100
+#define OV2680_REG_SOFT_RESET  0x0103
+
+#define OV2680_REG_CHIP_ID_HIGH0x300a
+#define OV2680_REG_CHIP_ID_LOW 0x300b
+
+#define OV2680_REG_R_MANUAL0x3503
+#define OV2680_REG_GAIN_PK 0x350a
+#define OV2680_REG_EXPOSURE_PK_HIGH0x3500
+#define OV2680_REG_TIMING_HTS  0x380c
+#define OV2680_REG_TIMING_VTS  0x380e
+#define OV2680_REG_FORMAT1 0x3820
+#define OV2680_REG_FORMAT2 0x3821
+
+#define OV2680_REG_ISP_CTRL00  0x5080
+
+#define OV2680_FRAME_RATE  30
+
+#define OV2680_REG_VALUE_8BIT  1
+#define OV2680_REG_VALUE_16BIT 2
+#define OV2680_REG_VALUE_24BIT 3
+
+enum ov2680_mode_id {
+   OV2680_MODE_QUXGA_800_600,
+   OV2680_MODE_720P_1280_720,
+   OV2680_MODE_UXGA_1600_1200,
+   OV2680_MODE_MAX,
+};
+
+struct reg_value {
+   u16 reg_addr;
+   u8 val;
+};
+
+struct ov2680_mode_info {
+   const char *name;
+   enum ov2680_mode_id id;
+   u32 width;
+   u32 height;
+   const struct reg_value *reg_data;
+   u32 reg_data_size;
+};
+
+struct ov2680_ctrls {
+   struct v4l2_ctrl_handler handler;
+   struct {
+   struct v4l2_ctrl *auto_exp;
+   struct v4l2_ctrl *exposure;
+   };
+   struct {
+   struct v4l2_ctrl *auto_gain;
+   struct v4l2_ctrl *gain;
+   };
+
+   struct v4l2_ctrl *hflip;
+   struct v4l2_ctrl *vflip;
+   struct v4l2_ctrl *test_pattern;
+};
+
+struct ov2680_dev {
+   struct i2c_client   *i2c_client;
+   struct v4l2_subdev  sd;
+
+   struct media_padpad;
+   struct clk  *xvclk;
+   u32 xvclk_freq;
+
+   struct gpio_desc*pwdn_gpio;
+   struct mutexlock; /* protect members */
+
+   boolmode_pending_changes;
+   boolis_enabled;
+   bool  

[PATCH v4 0/2] media: Introduce Omnivision OV2680 driver

[Rui Miguel Silva]

Add driver and bindings for the OV2680 2 megapixel CMOS 1/5" sensor, which has
a single MIPI lane interface and output format of 10-bit Raw RGB.

Features supported are described in PATCH 2/2.

v3->v4:
Sakari Ailus:
   - remove auto_{exposure|gain}_enable and direct call the set functions
   - add separe control sets to gain and exposure
   - fix number of controls allocated
   - check the exact frequency that it is supported

v2->v3:
Rob Herring:
- add Reviewed-by tag to dts PATCH 1/1

Sakari Ailus:
- align register values with bracket
- redone the {write|read}_reg i2c functions
- add bayer order handling with flip and mirror controls
- fix error path in probe release resources
- remove i2c_device_id and use probe_new

Myself:
- remove ; at the end of macros

v1->v2:
Fabio Estevam:
- s/OV5640/OV2680 in PATCH 1/2 changelog

Sakari Ailus:
- add description on endpoint properties in bindings
- add single endpoint in bindings
- drop OF dependency
- cleanup includes
- fix case in Color Bars
- remove frame rate selection
- 8/16/24 bit register access in the same transaction
- merge _reset and _soft_reset to _enable and rename it to power_on 
- _gain_set use only the gain value (drop & 0x7ff)
- _gain_get remove the (0x377)
- single write/read at _exposure_set/get use write_reg24/read_reg24
- move mode_set_direct to _mode_set
- _mode_set set auto exposure/gain based on ctrl value
- s_frame_interval equal to g_frame_interval
- use closest match from: v4l: common: Add a function to obtain best size 
from a list 
- check v4l2_ctrl_new_std return in _init

- fix gain manual value in auto_cluster

Cheers,
    Rui


Rui Miguel Silva (2):
  media: ov2680: dt: Add bindings for OV2680
  media: ov2680: Add Omnivision OV2680 sensor driver

 .../devicetree/bindings/media/i2c/ov2680.txt   |   40 +
 drivers/media/i2c/Kconfig  |   12 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov2680.c |  
 4 files changed, 1164 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt
 create mode 100644 drivers/media/i2c/ov2680.c

-- 
2.16.3

[PATCH v4 0/2] media: Introduce Omnivision OV2680 driver

[Rui Miguel Silva]

Add driver and bindings for the OV2680 2 megapixel CMOS 1/5" sensor, which has
a single MIPI lane interface and output format of 10-bit Raw RGB.

Features supported are described in PATCH 2/2.

v3->v4:
Sakari Ailus:
   - remove auto_{exposure|gain}_enable and direct call the set functions
   - add separe control sets to gain and exposure
   - fix number of controls allocated
   - check the exact frequency that it is supported

v2->v3:
Rob Herring:
- add Reviewed-by tag to dts PATCH 1/1

Sakari Ailus:
- align register values with bracket
- redone the {write|read}_reg i2c functions
- add bayer order handling with flip and mirror controls
- fix error path in probe release resources
- remove i2c_device_id and use probe_new

Myself:
- remove ; at the end of macros

v1->v2:
Fabio Estevam:
- s/OV5640/OV2680 in PATCH 1/2 changelog

Sakari Ailus:
- add description on endpoint properties in bindings
- add single endpoint in bindings
- drop OF dependency
- cleanup includes
- fix case in Color Bars
- remove frame rate selection
- 8/16/24 bit register access in the same transaction
- merge _reset and _soft_reset to _enable and rename it to power_on 
- _gain_set use only the gain value (drop & 0x7ff)
- _gain_get remove the (0x377)
- single write/read at _exposure_set/get use write_reg24/read_reg24
- move mode_set_direct to _mode_set
- _mode_set set auto exposure/gain based on ctrl value
- s_frame_interval equal to g_frame_interval
- use closest match from: v4l: common: Add a function to obtain best size 
from a list 
- check v4l2_ctrl_new_std return in _init

- fix gain manual value in auto_cluster

Cheers,
    Rui


Rui Miguel Silva (2):
  media: ov2680: dt: Add bindings for OV2680
  media: ov2680: Add Omnivision OV2680 sensor driver

 .../devicetree/bindings/media/i2c/ov2680.txt   |   40 +
 drivers/media/i2c/Kconfig  |   12 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov2680.c |  
 4 files changed, 1164 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt
 create mode 100644 drivers/media/i2c/ov2680.c

-- 
2.16.3

Re: [PATCH v3 2/2] media: ov2680: Add Omnivision OV2680 sensor driver

[Rui Miguel Silva]

Hi Sakari,
On Sat 24 Mar 2018 at 10:57, Sakari Ailus wrote:

Hi Rui,

I wanted to go through the code the last time and I'd have a few 
review

comments below...


Thanks for the review.



On Tue, Mar 13, 2018 at 11:33:11AM +, Rui Miguel Silva 
wrote:

...
+static int ov2680_gain_set(struct ov2680_dev *sensor, bool 
auto_gain)

+{
+   struct ov2680_ctrls *ctrls = >ctrls;
+   u32 gain;
+   int ret;
+
+   ret = ov2680_mod_reg(sensor, OV2680_REG_R_MANUAL, BIT(1),
+auto_gain ? 0 : BIT(1));
+   if (ret < 0)
+   return ret;
+
+   if (auto_gain || !ctrls->gain->is_new)
+   return 0;
+
+   gain = ctrls->gain->val;
+
+	ret = ov2680_write_reg16(sensor, OV2680_REG_GAIN_PK, 
gain);

+
+   return 0;
+}
+
+static int ov2680_gain_get(struct ov2680_dev *sensor)
+{
+   u32 gain;
+   int ret;
+
+	ret = ov2680_read_reg16(sensor, OV2680_REG_GAIN_PK, 
);

+   if (ret)
+   return ret;
+
+   return gain;
+}
+
+static int ov2680_auto_gain_enable(struct ov2680_dev *sensor)
+{
+   return ov2680_gain_set(sensor, true);
+}
+
+static int ov2680_auto_gain_disable(struct ov2680_dev *sensor)
+{
+   return ov2680_gain_set(sensor, false);
+}
+
+static int ov2680_exposure_set(struct ov2680_dev *sensor, bool 
auto_exp)

+{
+   struct ov2680_ctrls *ctrls = >ctrls;
+   u32 exp;
+   int ret;
+
+   ret = ov2680_mod_reg(sensor, OV2680_REG_R_MANUAL, BIT(0),
+auto_exp ? 0 : BIT(0));
+   if (ret < 0)
+   return ret;
+
+   if (auto_exp || !ctrls->exposure->is_new)
+   return 0;
+
+   exp = (u32)ctrls->exposure->val;
+   exp <<= 4;
+
+	return ov2680_write_reg24(sensor, 
OV2680_REG_EXPOSURE_PK_HIGH, exp);

+}
+
+static int ov2680_exposure_get(struct ov2680_dev *sensor)
+{
+   int ret;
+   u32 exp;
+
+	ret = ov2680_read_reg24(sensor, 
OV2680_REG_EXPOSURE_PK_HIGH, );

+   if (ret)
+   return ret;
+
+   return exp >> 4;
+}
+
+static int ov2680_auto_exposure_enable(struct ov2680_dev 
*sensor)

+{
+   return ov2680_exposure_set(sensor, true);
+}
+
+static int ov2680_auto_exposure_disable(struct ov2680_dev 
*sensor)

+{
+   return ov2680_exposure_set(sensor, false);
+}


I still think you could call the function actually doing the 
work, and pass
the bool parameter. That'd be much clearer. Please see the 
comments below,

too; they're related. Same for gain.


Ok, no problem, will change that in v4.



...


+static int ov2680_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
+{
+   struct v4l2_subdev *sd = ctrl_to_sd(ctrl);
+   struct ov2680_dev *sensor = to_ov2680_dev(sd);
+   int val;
+
+   if (!sensor->is_enabled)
+   return 0;
+
+   switch (ctrl->id) {
+   case V4L2_CID_AUTOGAIN:
+   if (!ctrl->val)
+   return 0;
+   val = ov2680_gain_get(sensor);
+   if (val < 0)
+   return val;
+   sensor->ctrls.gain->val = val;
+   break;
+   case V4L2_CID_EXPOSURE_AUTO:


I reckon the purpose of implementing volatile controls here 
would be to
provide the exposure and gain values to the user. But the 
controls here are
the ones enabling or disabling the automatic behaviour, not the 
value of

those settings themselves.


+   if (ctrl->val == V4L2_EXPOSURE_MANUAL)
+   return 0;
+   val = ov2680_exposure_get(sensor);
+   if (val < 0)
+   return val;
+   sensor->ctrls.exposure->val = val;
+   break;
+   }
+
+   return 0;
+}
+
+static int ov2680_s_ctrl(struct v4l2_ctrl *ctrl)
+{
+   struct v4l2_subdev *sd = ctrl_to_sd(ctrl);
+   struct ov2680_dev *sensor = to_ov2680_dev(sd);
+
+   if (!sensor->is_enabled)
+   return 0;
+
+   switch (ctrl->id) {
+   case V4L2_CID_AUTOGAIN:
+   return ov2680_gain_set(sensor, !!ctrl->val);
+   case V4L2_CID_EXPOSURE_AUTO:
+   return ov2680_exposure_set(sensor, !!ctrl->val);


With this, you can enable or disable automatic exposure and 
gain, but you

cannot manually set the values. Are such controls useful?

Unless I'm mistaken, exposure or gain are not settable, so you 
should make
them read-only controls. Or better, allow setting them if 
automatic control

is disabled.


Yeah, I could definitely change the values of exposure and gain 
also,
but that may came how the ctrl group work internally. But I will 
change

and add them.




+   case V4L2_CID_VFLIP:
+   if (sensor->is_streaming)
+   return -EBUSY;
+   if (ctrl->val)
+   return ov2680_vflip_enable(sensor);
+   else
+   return ov2680_vflip_disable(sen

Re: [PATCH v3 2/2] media: ov2680: Add Omnivision OV2680 sensor driver

[Rui Miguel Silva]

Hi Sakari,
On Sat 24 Mar 2018 at 10:57, Sakari Ailus wrote:

Hi Rui,

I wanted to go through the code the last time and I'd have a few 
review

comments below...


Thanks for the review.



On Tue, Mar 13, 2018 at 11:33:11AM +, Rui Miguel Silva 
wrote:

...
+static int ov2680_gain_set(struct ov2680_dev *sensor, bool 
auto_gain)

+{
+   struct ov2680_ctrls *ctrls = >ctrls;
+   u32 gain;
+   int ret;
+
+   ret = ov2680_mod_reg(sensor, OV2680_REG_R_MANUAL, BIT(1),
+auto_gain ? 0 : BIT(1));
+   if (ret < 0)
+   return ret;
+
+   if (auto_gain || !ctrls->gain->is_new)
+   return 0;
+
+   gain = ctrls->gain->val;
+
+	ret = ov2680_write_reg16(sensor, OV2680_REG_GAIN_PK, 
gain);

+
+   return 0;
+}
+
+static int ov2680_gain_get(struct ov2680_dev *sensor)
+{
+   u32 gain;
+   int ret;
+
+	ret = ov2680_read_reg16(sensor, OV2680_REG_GAIN_PK, 
);

+   if (ret)
+   return ret;
+
+   return gain;
+}
+
+static int ov2680_auto_gain_enable(struct ov2680_dev *sensor)
+{
+   return ov2680_gain_set(sensor, true);
+}
+
+static int ov2680_auto_gain_disable(struct ov2680_dev *sensor)
+{
+   return ov2680_gain_set(sensor, false);
+}
+
+static int ov2680_exposure_set(struct ov2680_dev *sensor, bool 
auto_exp)

+{
+   struct ov2680_ctrls *ctrls = >ctrls;
+   u32 exp;
+   int ret;
+
+   ret = ov2680_mod_reg(sensor, OV2680_REG_R_MANUAL, BIT(0),
+auto_exp ? 0 : BIT(0));
+   if (ret < 0)
+   return ret;
+
+   if (auto_exp || !ctrls->exposure->is_new)
+   return 0;
+
+   exp = (u32)ctrls->exposure->val;
+   exp <<= 4;
+
+	return ov2680_write_reg24(sensor, 
OV2680_REG_EXPOSURE_PK_HIGH, exp);

+}
+
+static int ov2680_exposure_get(struct ov2680_dev *sensor)
+{
+   int ret;
+   u32 exp;
+
+	ret = ov2680_read_reg24(sensor, 
OV2680_REG_EXPOSURE_PK_HIGH, );

+   if (ret)
+   return ret;
+
+   return exp >> 4;
+}
+
+static int ov2680_auto_exposure_enable(struct ov2680_dev 
*sensor)

+{
+   return ov2680_exposure_set(sensor, true);
+}
+
+static int ov2680_auto_exposure_disable(struct ov2680_dev 
*sensor)

+{
+   return ov2680_exposure_set(sensor, false);
+}


I still think you could call the function actually doing the 
work, and pass
the bool parameter. That'd be much clearer. Please see the 
comments below,

too; they're related. Same for gain.


Ok, no problem, will change that in v4.



...


+static int ov2680_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
+{
+   struct v4l2_subdev *sd = ctrl_to_sd(ctrl);
+   struct ov2680_dev *sensor = to_ov2680_dev(sd);
+   int val;
+
+   if (!sensor->is_enabled)
+   return 0;
+
+   switch (ctrl->id) {
+   case V4L2_CID_AUTOGAIN:
+   if (!ctrl->val)
+   return 0;
+   val = ov2680_gain_get(sensor);
+   if (val < 0)
+   return val;
+   sensor->ctrls.gain->val = val;
+   break;
+   case V4L2_CID_EXPOSURE_AUTO:


I reckon the purpose of implementing volatile controls here 
would be to
provide the exposure and gain values to the user. But the 
controls here are
the ones enabling or disabling the automatic behaviour, not the 
value of

those settings themselves.


+   if (ctrl->val == V4L2_EXPOSURE_MANUAL)
+   return 0;
+   val = ov2680_exposure_get(sensor);
+   if (val < 0)
+   return val;
+   sensor->ctrls.exposure->val = val;
+   break;
+   }
+
+   return 0;
+}
+
+static int ov2680_s_ctrl(struct v4l2_ctrl *ctrl)
+{
+   struct v4l2_subdev *sd = ctrl_to_sd(ctrl);
+   struct ov2680_dev *sensor = to_ov2680_dev(sd);
+
+   if (!sensor->is_enabled)
+   return 0;
+
+   switch (ctrl->id) {
+   case V4L2_CID_AUTOGAIN:
+   return ov2680_gain_set(sensor, !!ctrl->val);
+   case V4L2_CID_EXPOSURE_AUTO:
+   return ov2680_exposure_set(sensor, !!ctrl->val);


With this, you can enable or disable automatic exposure and 
gain, but you

cannot manually set the values. Are such controls useful?

Unless I'm mistaken, exposure or gain are not settable, so you 
should make
them read-only controls. Or better, allow setting them if 
automatic control

is disabled.


Yeah, I could definitely change the values of exposure and gain 
also,
but that may came how the ctrl group work internally. But I will 
change

and add them.




+   case V4L2_CID_VFLIP:
+   if (sensor->is_streaming)
+   return -EBUSY;
+   if (ctrl->val)
+   return ov2680_vflip_enable(sensor);
+   else
+   return ov2680_vflip_disable(sen

Re: [PATCH v3 2/2] media: ov2680: Add Omnivision OV2680 sensor driver

[Rui Miguel Silva]

Hi Sakari,
On Fri 16 Mar 2018 at 16:10, Sakari Ailus wrote:
On Thu, Mar 15, 2018 at 09:29:33AM +, Rui Miguel Silva 
wrote:

Hi,
On Wed 14 Mar 2018 at 19:39, kbuild test robot wrote:
> Hi Rui,
> 
> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on v4.16-rc4]

> [cannot apply to next-20180314]
> [if your patch is applied to the wrong git tree, please drop 
> us a note

> to help improve the system]
> 
> url: 
> https://github.com/0day-ci/linux/commits/Rui-Miguel-Silva/media-Introduce-Omnivision-OV2680-driver/20180315-020617

> config: sh-allmodconfig (attached as .config)
> compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
> reproduce:
> wget
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
> -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=sh
> 
> All errors (new ones prefixed by >>):
> 
>drivers/media/i2c/ov2680.c: In function 'ov2680_set_fmt':
> > > drivers/media/i2c/ov2680.c:713:9: error: implicit 
> > > declaration of

> > > function 'v4l2_find_nearest_size'; did you mean
> > > 'v4l2_find_nearest_format'?
> > > [-Werror=implicit-function-declaration]
>  mode = v4l2_find_nearest_size(ov2680_mode_data,
> ^~
> v4l2_find_nearest_format

As requested by maintainer this series depend on this patch 
[0], which
introduce this macro. I am not sure of the status of that patch 
though.


No need to worry about that, the sensor driver will just be 
merged after
the dependencies are in. Mauro said he'd handle the pull request 
early next

week.


Great, Many thanks for everything.

---
Cheers,
Rui

Re: [PATCH v3 2/2] media: ov2680: Add Omnivision OV2680 sensor driver

[Rui Miguel Silva]

Hi Sakari,
On Fri 16 Mar 2018 at 16:10, Sakari Ailus wrote:
On Thu, Mar 15, 2018 at 09:29:33AM +, Rui Miguel Silva 
wrote:

Hi,
On Wed 14 Mar 2018 at 19:39, kbuild test robot wrote:
> Hi Rui,
> 
> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on v4.16-rc4]

> [cannot apply to next-20180314]
> [if your patch is applied to the wrong git tree, please drop 
> us a note

> to help improve the system]
> 
> url: 
> https://github.com/0day-ci/linux/commits/Rui-Miguel-Silva/media-Introduce-Omnivision-OV2680-driver/20180315-020617

> config: sh-allmodconfig (attached as .config)
> compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
> reproduce:
> wget
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross
> -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=sh
> 
> All errors (new ones prefixed by >>):
> 
>drivers/media/i2c/ov2680.c: In function 'ov2680_set_fmt':
> > > drivers/media/i2c/ov2680.c:713:9: error: implicit 
> > > declaration of

> > > function 'v4l2_find_nearest_size'; did you mean
> > > 'v4l2_find_nearest_format'?
> > > [-Werror=implicit-function-declaration]
>  mode = v4l2_find_nearest_size(ov2680_mode_data,
> ^~
> v4l2_find_nearest_format

As requested by maintainer this series depend on this patch 
[0], which
introduce this macro. I am not sure of the status of that patch 
though.


No need to worry about that, the sensor driver will just be 
merged after
the dependencies are in. Mauro said he'd handle the pull request 
early next

week.


Great, Many thanks for everything.

---
Cheers,
Rui

Re: [PATCH v3 2/2] media: ov2680: Add Omnivision OV2680 sensor driver

[Rui Miguel Silva]

Hi,
On Wed 14 Mar 2018 at 19:39, kbuild test robot wrote:

Hi Rui,

I love your patch! Yet something to improve:

[auto build test ERROR on v4.16-rc4]
[cannot apply to next-20180314]
[if your patch is applied to the wrong git tree, please drop us 
a note to help improve the system]


url: 
https://github.com/0day-ci/linux/commits/Rui-Miguel-Silva/media-Introduce-Omnivision-OV2680-driver/20180315-020617

config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross 
-O ~/bin/make.cross

chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sh 


All errors (new ones prefixed by >>):

   drivers/media/i2c/ov2680.c: In function 'ov2680_set_fmt':
drivers/media/i2c/ov2680.c:713:9: error: implicit declaration 
of function 'v4l2_find_nearest_size'; did you mean 
'v4l2_find_nearest_format'? 
[-Werror=implicit-function-declaration]

 mode = v4l2_find_nearest_size(ov2680_mode_data,
^~
v4l2_find_nearest_format


As requested by maintainer this series depend on this patch [0], 
which
introduce this macro. I am not sure of the status of that patch 
though.


---
Cheers,
Rui

[0] https://patchwork.kernel.org/patch/10207087/

drivers/media/i2c/ov2680.c:714:41: error: 'width' undeclared 
(first use in this function)

  ARRAY_SIZE(ov2680_mode_data), width,
^
   drivers/media/i2c/ov2680.c:714:41: note: each undeclared 
   identifier is reported only once for each function it appears 
   in
drivers/media/i2c/ov2680.c:715:11: error: 'height' undeclared 
(first use in this function); did you mean 'hweight8'?

  height, fmt->width, fmt->height);
  ^~
  hweight8
   cc1: some warnings being treated as errors

vim +713 drivers/media/i2c/ov2680.c

   693  
   694  static int ov2680_set_fmt(struct v4l2_subdev *sd,
   695  struct 
   v4l2_subdev_pad_config *cfg,
   696  struct 
   v4l2_subdev_format *format)

   697  {
   698		struct ov2680_dev *sensor = 
   to_ov2680_dev(sd);
   699		struct v4l2_mbus_framefmt *fmt = 
   >format;

   700  const struct ov2680_mode_info *mode;
   701  int ret = 0;
   702  
   703  if (format->pad != 0)
   704  return -EINVAL;
   705  
   706  mutex_lock(>lock);
   707  
   708  if (sensor->is_streaming) {
   709  ret = -EBUSY;
   710  goto unlock;
   711  }
   712  
 > 713		mode = 
 > v4l2_find_nearest_size(ov2680_mode_data,
 > 714 
 > ARRAY_SIZE(ov2680_mode_data), width,
 > 715	  height, 
 > fmt->width, fmt->height);

   716  if (!mode) {
   717  ret = -EINVAL;
   718  goto unlock;
   719  }
   720  
   721		if (format->which == 
   V4L2_SUBDEV_FORMAT_TRY) {
   722			fmt = 
   v4l2_subdev_get_try_format(sd, cfg, 0);

   723  
   724  *fmt = format->format;
   725  goto unlock;
   726  }
   727  
   728  fmt->width = mode->width;
   729  fmt->height = mode->height;
   730  fmt->code = sensor->fmt.code;
   731  fmt->colorspace = sensor->fmt.colorspace;
   732  
   733  sensor->current_mode = mode;
   734  sensor->fmt = format->format;
   735  sensor->mode_pending_changes = true;
   736  
   737  unlock:
   738  mutex_unlock(>lock);
   739  
   740  return ret;
   741  }
   742  

---
0-DAY kernel test infrastructureOpen Source 
Technology Center
https://lists.01.org/pipermail/kbuild-all 
Intel Corporation

Re: [PATCH v3 2/2] media: ov2680: Add Omnivision OV2680 sensor driver

[Rui Miguel Silva]

Hi,
On Wed 14 Mar 2018 at 19:39, kbuild test robot wrote:

Hi Rui,

I love your patch! Yet something to improve:

[auto build test ERROR on v4.16-rc4]
[cannot apply to next-20180314]
[if your patch is applied to the wrong git tree, please drop us 
a note to help improve the system]


url: 
https://github.com/0day-ci/linux/commits/Rui-Miguel-Silva/media-Introduce-Omnivision-OV2680-driver/20180315-020617

config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross 
-O ~/bin/make.cross

chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sh 


All errors (new ones prefixed by >>):

   drivers/media/i2c/ov2680.c: In function 'ov2680_set_fmt':
drivers/media/i2c/ov2680.c:713:9: error: implicit declaration 
of function 'v4l2_find_nearest_size'; did you mean 
'v4l2_find_nearest_format'? 
[-Werror=implicit-function-declaration]

 mode = v4l2_find_nearest_size(ov2680_mode_data,
^~
v4l2_find_nearest_format


As requested by maintainer this series depend on this patch [0], 
which
introduce this macro. I am not sure of the status of that patch 
though.


---
Cheers,
Rui

[0] https://patchwork.kernel.org/patch/10207087/

drivers/media/i2c/ov2680.c:714:41: error: 'width' undeclared 
(first use in this function)

  ARRAY_SIZE(ov2680_mode_data), width,
^
   drivers/media/i2c/ov2680.c:714:41: note: each undeclared 
   identifier is reported only once for each function it appears 
   in
drivers/media/i2c/ov2680.c:715:11: error: 'height' undeclared 
(first use in this function); did you mean 'hweight8'?

  height, fmt->width, fmt->height);
  ^~
  hweight8
   cc1: some warnings being treated as errors

vim +713 drivers/media/i2c/ov2680.c

   693  
   694  static int ov2680_set_fmt(struct v4l2_subdev *sd,
   695  struct 
   v4l2_subdev_pad_config *cfg,
   696  struct 
   v4l2_subdev_format *format)

   697  {
   698		struct ov2680_dev *sensor = 
   to_ov2680_dev(sd);
   699		struct v4l2_mbus_framefmt *fmt = 
   >format;

   700  const struct ov2680_mode_info *mode;
   701  int ret = 0;
   702  
   703  if (format->pad != 0)
   704  return -EINVAL;
   705  
   706  mutex_lock(>lock);
   707  
   708  if (sensor->is_streaming) {
   709  ret = -EBUSY;
   710  goto unlock;
   711  }
   712  
 > 713		mode = 
 > v4l2_find_nearest_size(ov2680_mode_data,
 > 714 
 > ARRAY_SIZE(ov2680_mode_data), width,
 > 715	  height, 
 > fmt->width, fmt->height);

   716  if (!mode) {
   717  ret = -EINVAL;
   718  goto unlock;
   719  }
   720  
   721		if (format->which == 
   V4L2_SUBDEV_FORMAT_TRY) {
   722			fmt = 
   v4l2_subdev_get_try_format(sd, cfg, 0);

   723  
   724  *fmt = format->format;
   725  goto unlock;
   726  }
   727  
   728  fmt->width = mode->width;
   729  fmt->height = mode->height;
   730  fmt->code = sensor->fmt.code;
   731  fmt->colorspace = sensor->fmt.colorspace;
   732  
   733  sensor->current_mode = mode;
   734  sensor->fmt = format->format;
   735  sensor->mode_pending_changes = true;
   736  
   737  unlock:
   738  mutex_unlock(>lock);
   739  
   740  return ret;
   741  }
   742  

---
0-DAY kernel test infrastructureOpen Source 
Technology Center
https://lists.01.org/pipermail/kbuild-all 
Intel Corporation

[PATCH v3 2/2] media: ov2680: Add Omnivision OV2680 sensor driver

[Rui Miguel Silva]

This patch adds V4L2 sub-device driver for OV2680 image sensor.
The OV2680 is a 1/5" CMOS color sensor from Omnivision.
Supports output format: 10-bit Raw RGB.
The OV2680 has a single lane MIPI interface.

The driver exposes following V4L2 controls:
- auto/manual exposure,
- exposure,
- auto/manual gain,
- gain,
- horizontal/vertical flip,
- test pattern menu.
Supported resolution are only: QUXGA, 720P, UXGA.

Signed-off-by: Rui Miguel Silva <rui.si...@linaro.org>
---
 drivers/media/i2c/Kconfig  |   12 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov2680.c | 1130 
 3 files changed, 1143 insertions(+)
 create mode 100644 drivers/media/i2c/ov2680.c

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 9f18cd296841..39dc9f236ffa 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -586,6 +586,18 @@ config VIDEO_OV2659
  To compile this driver as a module, choose M here: the
  module will be called ov2659.
 
+config VIDEO_OV2680
+   tristate "OmniVision OV2680 sensor support"
+   depends on GPIOLIB && VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API
+   depends on MEDIA_CAMERA_SUPPORT
+   select V4L2_FWNODE
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the OmniVision
+ OV2680 camera sensor with a MIPI CSI-2 interface.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ov2680.
+
 config VIDEO_OV5640
tristate "OmniVision OV5640 sensor support"
depends on OF
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index c0f94cd8d56d..d0aba4d37b8d 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o
 obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
 obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
 obj-$(CONFIG_VIDEO_OV2640) += ov2640.o
+obj-$(CONFIG_VIDEO_OV2680) += ov2680.o
 obj-$(CONFIG_VIDEO_OV5640) += ov5640.o
 obj-$(CONFIG_VIDEO_OV5645) += ov5645.o
 obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c
new file mode 100644
index ..a17971ce71bc
--- /dev/null
+++ b/drivers/media/i2c/ov2680.c
@@ -0,0 +1,1130 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Omnivision OV2680 CMOS Image Sensor driver
+ *
+ * Copyright (C) 2018 Linaro Ltd
+ *
+ * Based on OV5640 Sensor Driver
+ * Copyright (C) 2011-2013 Freescale Semiconductor, Inc. All Rights Reserved.
+ * Copyright (C) 2014-2017 Mentor Graphics Inc.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#define OV2680_XVCLK_MIN   600
+#define OV2680_XVCLK_MAX   2400
+
+#define OV2680_CHIP_ID 0x2680
+
+#define OV2680_REG_STREAM_CTRL 0x0100
+#define OV2680_REG_SOFT_RESET  0x0103
+
+#define OV2680_REG_CHIP_ID_HIGH0x300a
+#define OV2680_REG_CHIP_ID_LOW 0x300b
+
+#define OV2680_REG_R_MANUAL0x3503
+#define OV2680_REG_GAIN_PK 0x350a
+#define OV2680_REG_EXPOSURE_PK_HIGH0x3500
+#define OV2680_REG_TIMING_HTS  0x380c
+#define OV2680_REG_TIMING_VTS  0x380e
+#define OV2680_REG_FORMAT1 0x3820
+#define OV2680_REG_FORMAT2 0x3821
+
+#define OV2680_REG_ISP_CTRL00  0x5080
+
+#define OV2680_FRAME_RATE  30
+
+#define OV2680_REG_VALUE_8BIT  1
+#define OV2680_REG_VALUE_16BIT 2
+#define OV2680_REG_VALUE_24BIT 3
+
+enum ov2680_mode_id {
+   OV2680_MODE_QUXGA_800_600,
+   OV2680_MODE_720P_1280_720,
+   OV2680_MODE_UXGA_1600_1200,
+   OV2680_MODE_MAX,
+};
+
+struct reg_value {
+   u16 reg_addr;
+   u8 val;
+};
+
+struct ov2680_mode_info {
+   const char *name;
+   enum ov2680_mode_id id;
+   u32 width;
+   u32 height;
+   const struct reg_value *reg_data;
+   u32 reg_data_size;
+};
+
+struct ov2680_ctrls {
+   struct v4l2_ctrl_handler handler;
+   struct {
+   struct v4l2_ctrl *auto_exp;
+   struct v4l2_ctrl *exposure;
+   };
+   struct {
+   struct v4l2_ctrl *auto_gain;
+   struct v4l2_ctrl *gain;
+   };
+
+   struct v4l2_ctrl *hflip;
+   struct v4l2_ctrl *vflip;
+   struct v4l2_ctrl *test_pattern;
+};
+
+struct ov2680_dev {
+   struct i2c_client   *i2c_client;
+   struct v4l2_subdev  sd;
+
+   struct media_padpad;
+   struct clk  *xvclk;
+   u32 xvclk_freq;
+
+   struct gpio_desc*pwdn_gpio;
+   struct mutexlock; /* protect members */
+
+   boolmode_pending_changes;
+   b

[PATCH v3 2/2] media: ov2680: Add Omnivision OV2680 sensor driver

[Rui Miguel Silva]

This patch adds V4L2 sub-device driver for OV2680 image sensor.
The OV2680 is a 1/5" CMOS color sensor from Omnivision.
Supports output format: 10-bit Raw RGB.
The OV2680 has a single lane MIPI interface.

The driver exposes following V4L2 controls:
- auto/manual exposure,
- exposure,
- auto/manual gain,
- gain,
- horizontal/vertical flip,
- test pattern menu.
Supported resolution are only: QUXGA, 720P, UXGA.

Signed-off-by: Rui Miguel Silva 
---
 drivers/media/i2c/Kconfig  |   12 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov2680.c | 1130 
 3 files changed, 1143 insertions(+)
 create mode 100644 drivers/media/i2c/ov2680.c

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 9f18cd296841..39dc9f236ffa 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -586,6 +586,18 @@ config VIDEO_OV2659
  To compile this driver as a module, choose M here: the
  module will be called ov2659.
 
+config VIDEO_OV2680
+   tristate "OmniVision OV2680 sensor support"
+   depends on GPIOLIB && VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API
+   depends on MEDIA_CAMERA_SUPPORT
+   select V4L2_FWNODE
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the OmniVision
+ OV2680 camera sensor with a MIPI CSI-2 interface.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ov2680.
+
 config VIDEO_OV5640
tristate "OmniVision OV5640 sensor support"
depends on OF
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index c0f94cd8d56d..d0aba4d37b8d 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o
 obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
 obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
 obj-$(CONFIG_VIDEO_OV2640) += ov2640.o
+obj-$(CONFIG_VIDEO_OV2680) += ov2680.o
 obj-$(CONFIG_VIDEO_OV5640) += ov5640.o
 obj-$(CONFIG_VIDEO_OV5645) += ov5645.o
 obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c
new file mode 100644
index ..a17971ce71bc
--- /dev/null
+++ b/drivers/media/i2c/ov2680.c
@@ -0,0 +1,1130 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Omnivision OV2680 CMOS Image Sensor driver
+ *
+ * Copyright (C) 2018 Linaro Ltd
+ *
+ * Based on OV5640 Sensor Driver
+ * Copyright (C) 2011-2013 Freescale Semiconductor, Inc. All Rights Reserved.
+ * Copyright (C) 2014-2017 Mentor Graphics Inc.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#define OV2680_XVCLK_MIN   600
+#define OV2680_XVCLK_MAX   2400
+
+#define OV2680_CHIP_ID 0x2680
+
+#define OV2680_REG_STREAM_CTRL 0x0100
+#define OV2680_REG_SOFT_RESET  0x0103
+
+#define OV2680_REG_CHIP_ID_HIGH0x300a
+#define OV2680_REG_CHIP_ID_LOW 0x300b
+
+#define OV2680_REG_R_MANUAL0x3503
+#define OV2680_REG_GAIN_PK 0x350a
+#define OV2680_REG_EXPOSURE_PK_HIGH0x3500
+#define OV2680_REG_TIMING_HTS  0x380c
+#define OV2680_REG_TIMING_VTS  0x380e
+#define OV2680_REG_FORMAT1 0x3820
+#define OV2680_REG_FORMAT2 0x3821
+
+#define OV2680_REG_ISP_CTRL00  0x5080
+
+#define OV2680_FRAME_RATE  30
+
+#define OV2680_REG_VALUE_8BIT  1
+#define OV2680_REG_VALUE_16BIT 2
+#define OV2680_REG_VALUE_24BIT 3
+
+enum ov2680_mode_id {
+   OV2680_MODE_QUXGA_800_600,
+   OV2680_MODE_720P_1280_720,
+   OV2680_MODE_UXGA_1600_1200,
+   OV2680_MODE_MAX,
+};
+
+struct reg_value {
+   u16 reg_addr;
+   u8 val;
+};
+
+struct ov2680_mode_info {
+   const char *name;
+   enum ov2680_mode_id id;
+   u32 width;
+   u32 height;
+   const struct reg_value *reg_data;
+   u32 reg_data_size;
+};
+
+struct ov2680_ctrls {
+   struct v4l2_ctrl_handler handler;
+   struct {
+   struct v4l2_ctrl *auto_exp;
+   struct v4l2_ctrl *exposure;
+   };
+   struct {
+   struct v4l2_ctrl *auto_gain;
+   struct v4l2_ctrl *gain;
+   };
+
+   struct v4l2_ctrl *hflip;
+   struct v4l2_ctrl *vflip;
+   struct v4l2_ctrl *test_pattern;
+};
+
+struct ov2680_dev {
+   struct i2c_client   *i2c_client;
+   struct v4l2_subdev  sd;
+
+   struct media_padpad;
+   struct clk  *xvclk;
+   u32 xvclk_freq;
+
+   struct gpio_desc*pwdn_gpio;
+   struct mutexlock; /* protect members */
+
+   boolmode_pending_changes;
+   bool   

[PATCH v3 1/2] media: ov2680: dt: Add bindings for OV2680

[Rui Miguel Silva]

Add device tree binding documentation for the OV2680 camera sensor.

Reviewed-by: Rob Herring <r...@kernel.org>
CC: devicet...@vger.kernel.org
Signed-off-by: Rui Miguel Silva <rui.si...@linaro.org>
---
 .../devicetree/bindings/media/i2c/ov2680.txt   | 40 ++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/ov2680.txt 
b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
new file mode 100644
index ..0e29f1a113c0
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
@@ -0,0 +1,40 @@
+* Omnivision OV2680 MIPI CSI-2 sensor
+
+Required Properties:
+- compatible: should be "ovti,ov2680".
+- clocks: reference to the xvclk input clock.
+- clock-names: should be "xvclk".
+
+Optional Properties:
+- powerdown-gpios: reference to the GPIO connected to the powerdown pin,
+if any. This is an active high signal to the OV2680.
+
+The device node must contain one 'port' child node for its digital output
+video port, and this port must have a single endpoint in accordance with
+ the video interface bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Endpoint node required properties for CSI-2 connection are:
+- remote-endpoint: a phandle to the bus receiver's endpoint node.
+- clock-lanes: should be set to <0> (clock lane on hardware lane 0).
+- data-lanes: should be set to <1> (one CSI-2 lane supported).
+ 
+Example:
+
+ {
+   ov2680: camera-sensor@36 {
+   compatible = "ovti,ov2680";
+   reg = <0x36>;
+   clocks = <>;
+   clock-names = "xvclk";
+   powerdown-gpios = < 3 GPIO_ACTIVE_HIGH>;
+
+   port {
+   ov2680_mipi_ep: endpoint {
+   remote-endpoint = <_sensor_ep>;
+   clock-lanes = <0>;
+   data-lanes = <1>;
+   };
+   };
+   };
+};
-- 
2.16.2

[PATCH v3 1/2] media: ov2680: dt: Add bindings for OV2680

[Rui Miguel Silva]

Add device tree binding documentation for the OV2680 camera sensor.

Reviewed-by: Rob Herring 
CC: devicet...@vger.kernel.org
Signed-off-by: Rui Miguel Silva 
---
 .../devicetree/bindings/media/i2c/ov2680.txt   | 40 ++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/ov2680.txt 
b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
new file mode 100644
index ..0e29f1a113c0
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
@@ -0,0 +1,40 @@
+* Omnivision OV2680 MIPI CSI-2 sensor
+
+Required Properties:
+- compatible: should be "ovti,ov2680".
+- clocks: reference to the xvclk input clock.
+- clock-names: should be "xvclk".
+
+Optional Properties:
+- powerdown-gpios: reference to the GPIO connected to the powerdown pin,
+if any. This is an active high signal to the OV2680.
+
+The device node must contain one 'port' child node for its digital output
+video port, and this port must have a single endpoint in accordance with
+ the video interface bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Endpoint node required properties for CSI-2 connection are:
+- remote-endpoint: a phandle to the bus receiver's endpoint node.
+- clock-lanes: should be set to <0> (clock lane on hardware lane 0).
+- data-lanes: should be set to <1> (one CSI-2 lane supported).
+ 
+Example:
+
+ {
+   ov2680: camera-sensor@36 {
+   compatible = "ovti,ov2680";
+   reg = <0x36>;
+   clocks = <>;
+   clock-names = "xvclk";
+   powerdown-gpios = < 3 GPIO_ACTIVE_HIGH>;
+
+   port {
+   ov2680_mipi_ep: endpoint {
+   remote-endpoint = <_sensor_ep>;
+   clock-lanes = <0>;
+   data-lanes = <1>;
+   };
+   };
+   };
+};
-- 
2.16.2

[PATCH v3 0/2] media: Introduce Omnivision OV2680 driver

[Rui Miguel Silva]

Add driver and bindings for the OV2680 2 megapixel CMOS 1/5" sensor, which has
a single MIPI lane interface and output format of 10-bit Raw RGB.

Features supported are described in PATCH 2/2.

v2->v3:
Rob Herring:
- add Reviewed-by tag to dts PATCH 1/1

Sakari Ailus:
- align register values with bracket
- redone the {write|read}_reg i2c functions
- add bayer order handling with flip and mirror controls
- fix error path in probe release resources
- remove i2c_device_id and use probe_new

Myself:
- remove ; at the end of macros

v1->v2:
Fabio Estevam:
- s/OV5640/OV2680 in PATCH 1/2 changelog

Sakari Ailus:
- add description on endpoint properties in bindings
- add single endpoint in bindings
- drop OF dependency
- cleanup includes
- fix case in Color Bars
- remove frame rate selection
- 8/16/24 bit register access in the same transaction
- merge _reset and _soft_reset to _enable and rename it to power_on 
- _gain_set use only the gain value (drop & 0x7ff)
- _gain_get remove the (0x377)
- single write/read at _exposure_set/get use write_reg24/read_reg24
- move mode_set_direct to _mode_set
- _mode_set set auto exposure/gain based on ctrl value
- s_frame_interval equal to g_frame_interval
- use closest match from: v4l: common: Add a function to obtain best size 
from a list 
- check v4l2_ctrl_new_std return in _init

- fix gain manual value in auto_cluster

Cheers,
Rui

Rui Miguel Silva (2):
  media: ov2680: dt: Add bindings for OV2680
  media: ov2680: Add Omnivision OV2680 sensor driver

 .../devicetree/bindings/media/i2c/ov2680.txt   |   40 +
 drivers/media/i2c/Kconfig  |   12 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov2680.c | 1130 
 4 files changed, 1183 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt
 create mode 100644 drivers/media/i2c/ov2680.c

-- 
2.16.2

[PATCH v3 0/2] media: Introduce Omnivision OV2680 driver

[Rui Miguel Silva]

Add driver and bindings for the OV2680 2 megapixel CMOS 1/5" sensor, which has
a single MIPI lane interface and output format of 10-bit Raw RGB.

Features supported are described in PATCH 2/2.

v2->v3:
Rob Herring:
- add Reviewed-by tag to dts PATCH 1/1

Sakari Ailus:
- align register values with bracket
- redone the {write|read}_reg i2c functions
- add bayer order handling with flip and mirror controls
- fix error path in probe release resources
- remove i2c_device_id and use probe_new

Myself:
- remove ; at the end of macros

v1->v2:
Fabio Estevam:
- s/OV5640/OV2680 in PATCH 1/2 changelog

Sakari Ailus:
- add description on endpoint properties in bindings
- add single endpoint in bindings
- drop OF dependency
- cleanup includes
- fix case in Color Bars
- remove frame rate selection
- 8/16/24 bit register access in the same transaction
- merge _reset and _soft_reset to _enable and rename it to power_on 
- _gain_set use only the gain value (drop & 0x7ff)
- _gain_get remove the (0x377)
- single write/read at _exposure_set/get use write_reg24/read_reg24
- move mode_set_direct to _mode_set
- _mode_set set auto exposure/gain based on ctrl value
- s_frame_interval equal to g_frame_interval
- use closest match from: v4l: common: Add a function to obtain best size 
from a list 
- check v4l2_ctrl_new_std return in _init

- fix gain manual value in auto_cluster

Cheers,
Rui

Rui Miguel Silva (2):
  media: ov2680: dt: Add bindings for OV2680
  media: ov2680: Add Omnivision OV2680 sensor driver

 .../devicetree/bindings/media/i2c/ov2680.txt   |   40 +
 drivers/media/i2c/Kconfig  |   12 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov2680.c | 1130 
 4 files changed, 1183 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt
 create mode 100644 drivers/media/i2c/ov2680.c

-- 
2.16.2

Re: [PATCH v2 2/2] media: ov2680: Add Omnivision OV2680 sensor driver

[Rui Miguel Silva]

Hi Sakari,
Thanks for the review...

On Fri 09 Mar 2018 at 09:21, Sakari Ailus wrote:

Hi Miguel,

Thanks for the update.

On Wed, Feb 28, 2018 at 03:27:23PM +, Rui Miguel Silva 
wrote:

This patch adds V4L2 sub-device driver for OV2680 image sensor.
The OV2680 is a 1/5" CMOS color sensor from Omnivision.
Supports output format: 10-bit Raw RGB.
The OV2680 has a single lane MIPI interface.

The driver exposes following V4L2 controls:
- auto/manual exposure,
- exposure,
- auto/manual gain,
- gain,
- horizontal/vertical flip,
- test pattern menu.
Supported resolution are only: QUXGA, 720P, UXGA.

Signed-off-by: Rui Miguel Silva <rui.si...@linaro.org>
---
 drivers/media/i2c/Kconfig  |   12 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov2680.c | 1094 
 

 3 files changed, 1107 insertions(+)
 create mode 100644 drivers/media/i2c/ov2680.c

diff --git a/drivers/media/i2c/Kconfig 
b/drivers/media/i2c/Kconfig

index 9f18cd296841..39dc9f236ffa 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -586,6 +586,18 @@ config VIDEO_OV2659
  To compile this driver as a module, choose M here: the
  module will be called ov2659.
 
+config VIDEO_OV2680

+   tristate "OmniVision OV2680 sensor support"
+	depends on GPIOLIB && VIDEO_V4L2 && I2C && 
VIDEO_V4L2_SUBDEV_API

+   depends on MEDIA_CAMERA_SUPPORT
+   select V4L2_FWNODE
+   ---help---
+	  This is a Video4Linux2 sensor-level driver for the 
OmniVision

+ OV2680 camera sensor with a MIPI CSI-2 interface.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ov2680.
+
 config VIDEO_OV5640
tristate "OmniVision OV5640 sensor support"
depends on OF
diff --git a/drivers/media/i2c/Makefile 
b/drivers/media/i2c/Makefile

index c0f94cd8d56d..d0aba4d37b8d 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += 
sony-btf-mpx.o

 obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
 obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
 obj-$(CONFIG_VIDEO_OV2640) += ov2640.o
+obj-$(CONFIG_VIDEO_OV2680) += ov2680.o
 obj-$(CONFIG_VIDEO_OV5640) += ov5640.o
 obj-$(CONFIG_VIDEO_OV5645) += ov5645.o
 obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
diff --git a/drivers/media/i2c/ov2680.c 
b/drivers/media/i2c/ov2680.c

new file mode 100644
index ..78f2be27a8f5
--- /dev/null
+++ b/drivers/media/i2c/ov2680.c
@@ -0,0 +1,1094 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Omnivision OV2680 CMOS Image Sensor driver
+ *
+ * Copyright (C) 2018 Linaro Ltd
+ *
+ * Based on OV5640 Sensor Driver
+ * Copyright (C) 2011-2013 Freescale Semiconductor, Inc. All 
Rights Reserved.

+ * Copyright (C) 2014-2017 Mentor Graphics Inc.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#define OV2680_XVCLK_MIN   600
+#define OV2680_XVCLK_MAX   2400
+
+#define OV2680_CHIP_ID 0x2680
+
+#define OV2680_REG_STREAM_CTRL 0x0100
+#define OV2680_REG_SOFT_RESET  0x0103
+
+#define OV2680_REG_CHIP_ID_HIGH0x300a
+#define OV2680_REG_CHIP_ID_LOW 0x300b
+
+#define OV2680_REG_R_MANUAL0x3503
+#define OV2680_REG_GAIN_PK 0x350a
+#define OV2680_REG_EXPOSURE_PK_HIGH0x3500
+#define OV2680_REG_TIMING_HTS  0x380c
+#define OV2680_REG_TIMING_VTS  0x380e
+#define OV2680_REG_FORMAT1 0x3820
+#define OV2680_REG_FORMAT2 0x3821
+
+#define OV2680_REG_ISP_CTRL00  0x5080
+
+#define OV2680_FRAME_RATE  30
+
+#define OV2680_REG_VALUE_8BIT  1
+#define OV2680_REG_VALUE_16BIT 2
+#define OV2680_REG_VALUE_24BIT 3
+
+enum ov2680_mode_id {
+   OV2680_MODE_QUXGA_800_600,
+   OV2680_MODE_720P_1280_720,
+   OV2680_MODE_UXGA_1600_1200,
+   OV2680_MODE_MAX,
+};
+
+struct reg_value {
+   u16 reg_addr;
+   u8 val;
+};
+
+struct ov2680_mode_info {
+   const char *name;
+   enum ov2680_mode_id id;
+   u32 width;
+   u32 height;
+   const struct reg_value *reg_data;
+   u32 reg_data_size;
+};
+
+struct ov2680_ctrls {
+   struct v4l2_ctrl_handler handler;
+   struct {
+   struct v4l2_ctrl *auto_exp;
+   struct v4l2_ctrl *exposure;
+   };
+   struct {
+   struct v4l2_ctrl *auto_gain;
+   struct v4l2_ctrl *gain;
+   };
+
+   struct v4l2_ctrl *hflip;
+   struct v4l2_ctrl *vflip;
+   struct v4l2_ctrl *test_pattern;
+};
+
+struct ov2680_dev {
+   struct i2c_client   *i2c_client;
+   struct v4l2_subdev  sd;
+
+   struct media_padpad;
+   struct clk  *xvclk;
+   u32 xvclk_freq;
+
+   struct gpi

Re: [PATCH v2 2/2] media: ov2680: Add Omnivision OV2680 sensor driver

[Rui Miguel Silva]

Hi Sakari,
Thanks for the review...

On Fri 09 Mar 2018 at 09:21, Sakari Ailus wrote:

Hi Miguel,

Thanks for the update.

On Wed, Feb 28, 2018 at 03:27:23PM +, Rui Miguel Silva 
wrote:

This patch adds V4L2 sub-device driver for OV2680 image sensor.
The OV2680 is a 1/5" CMOS color sensor from Omnivision.
Supports output format: 10-bit Raw RGB.
The OV2680 has a single lane MIPI interface.

The driver exposes following V4L2 controls:
- auto/manual exposure,
- exposure,
- auto/manual gain,
- gain,
- horizontal/vertical flip,
- test pattern menu.
Supported resolution are only: QUXGA, 720P, UXGA.

Signed-off-by: Rui Miguel Silva 
---
 drivers/media/i2c/Kconfig  |   12 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov2680.c | 1094 
 

 3 files changed, 1107 insertions(+)
 create mode 100644 drivers/media/i2c/ov2680.c

diff --git a/drivers/media/i2c/Kconfig 
b/drivers/media/i2c/Kconfig

index 9f18cd296841..39dc9f236ffa 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -586,6 +586,18 @@ config VIDEO_OV2659
  To compile this driver as a module, choose M here: the
  module will be called ov2659.
 
+config VIDEO_OV2680

+   tristate "OmniVision OV2680 sensor support"
+	depends on GPIOLIB && VIDEO_V4L2 && I2C && 
VIDEO_V4L2_SUBDEV_API

+   depends on MEDIA_CAMERA_SUPPORT
+   select V4L2_FWNODE
+   ---help---
+	  This is a Video4Linux2 sensor-level driver for the 
OmniVision

+ OV2680 camera sensor with a MIPI CSI-2 interface.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ov2680.
+
 config VIDEO_OV5640
tristate "OmniVision OV5640 sensor support"
depends on OF
diff --git a/drivers/media/i2c/Makefile 
b/drivers/media/i2c/Makefile

index c0f94cd8d56d..d0aba4d37b8d 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += 
sony-btf-mpx.o

 obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
 obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
 obj-$(CONFIG_VIDEO_OV2640) += ov2640.o
+obj-$(CONFIG_VIDEO_OV2680) += ov2680.o
 obj-$(CONFIG_VIDEO_OV5640) += ov5640.o
 obj-$(CONFIG_VIDEO_OV5645) += ov5645.o
 obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
diff --git a/drivers/media/i2c/ov2680.c 
b/drivers/media/i2c/ov2680.c

new file mode 100644
index ..78f2be27a8f5
--- /dev/null
+++ b/drivers/media/i2c/ov2680.c
@@ -0,0 +1,1094 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Omnivision OV2680 CMOS Image Sensor driver
+ *
+ * Copyright (C) 2018 Linaro Ltd
+ *
+ * Based on OV5640 Sensor Driver
+ * Copyright (C) 2011-2013 Freescale Semiconductor, Inc. All 
Rights Reserved.

+ * Copyright (C) 2014-2017 Mentor Graphics Inc.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#define OV2680_XVCLK_MIN   600
+#define OV2680_XVCLK_MAX   2400
+
+#define OV2680_CHIP_ID 0x2680
+
+#define OV2680_REG_STREAM_CTRL 0x0100
+#define OV2680_REG_SOFT_RESET  0x0103
+
+#define OV2680_REG_CHIP_ID_HIGH0x300a
+#define OV2680_REG_CHIP_ID_LOW 0x300b
+
+#define OV2680_REG_R_MANUAL0x3503
+#define OV2680_REG_GAIN_PK 0x350a
+#define OV2680_REG_EXPOSURE_PK_HIGH0x3500
+#define OV2680_REG_TIMING_HTS  0x380c
+#define OV2680_REG_TIMING_VTS  0x380e
+#define OV2680_REG_FORMAT1 0x3820
+#define OV2680_REG_FORMAT2 0x3821
+
+#define OV2680_REG_ISP_CTRL00  0x5080
+
+#define OV2680_FRAME_RATE  30
+
+#define OV2680_REG_VALUE_8BIT  1
+#define OV2680_REG_VALUE_16BIT 2
+#define OV2680_REG_VALUE_24BIT 3
+
+enum ov2680_mode_id {
+   OV2680_MODE_QUXGA_800_600,
+   OV2680_MODE_720P_1280_720,
+   OV2680_MODE_UXGA_1600_1200,
+   OV2680_MODE_MAX,
+};
+
+struct reg_value {
+   u16 reg_addr;
+   u8 val;
+};
+
+struct ov2680_mode_info {
+   const char *name;
+   enum ov2680_mode_id id;
+   u32 width;
+   u32 height;
+   const struct reg_value *reg_data;
+   u32 reg_data_size;
+};
+
+struct ov2680_ctrls {
+   struct v4l2_ctrl_handler handler;
+   struct {
+   struct v4l2_ctrl *auto_exp;
+   struct v4l2_ctrl *exposure;
+   };
+   struct {
+   struct v4l2_ctrl *auto_gain;
+   struct v4l2_ctrl *gain;
+   };
+
+   struct v4l2_ctrl *hflip;
+   struct v4l2_ctrl *vflip;
+   struct v4l2_ctrl *test_pattern;
+};
+
+struct ov2680_dev {
+   struct i2c_client   *i2c_client;
+   struct v4l2_subdev  sd;
+
+   struct media_padpad;
+   struct clk  *xvclk;
+   u32 xvclk_freq;
+
+   struct gpio_desc*pwd

[PATCH v2 1/2] media: ov2680: dt: Add bindings for OV2680

[Rui Miguel Silva]

Add device tree binding documentation for the OV2680 camera sensor.

CC: devicet...@vger.kernel.org
Signed-off-by: Rui Miguel Silva <rui.si...@linaro.org>
---
 .../devicetree/bindings/media/i2c/ov2680.txt   | 40 ++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/ov2680.txt 
b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
new file mode 100644
index ..0e29f1a113c0
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
@@ -0,0 +1,40 @@
+* Omnivision OV2680 MIPI CSI-2 sensor
+
+Required Properties:
+- compatible: should be "ovti,ov2680".
+- clocks: reference to the xvclk input clock.
+- clock-names: should be "xvclk".
+
+Optional Properties:
+- powerdown-gpios: reference to the GPIO connected to the powerdown pin,
+if any. This is an active high signal to the OV2680.
+
+The device node must contain one 'port' child node for its digital output
+video port, and this port must have a single endpoint in accordance with
+ the video interface bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Endpoint node required properties for CSI-2 connection are:
+- remote-endpoint: a phandle to the bus receiver's endpoint node.
+- clock-lanes: should be set to <0> (clock lane on hardware lane 0).
+- data-lanes: should be set to <1> (one CSI-2 lane supported).
+ 
+Example:
+
+ {
+   ov2680: camera-sensor@36 {
+   compatible = "ovti,ov2680";
+   reg = <0x36>;
+   clocks = <>;
+   clock-names = "xvclk";
+   powerdown-gpios = < 3 GPIO_ACTIVE_HIGH>;
+
+   port {
+   ov2680_mipi_ep: endpoint {
+   remote-endpoint = <_sensor_ep>;
+   clock-lanes = <0>;
+   data-lanes = <1>;
+   };
+   };
+   };
+};
-- 
2.16.2

[PATCH v2 2/2] media: ov2680: Add Omnivision OV2680 sensor driver

[Rui Miguel Silva]

This patch adds V4L2 sub-device driver for OV2680 image sensor.
The OV2680 is a 1/5" CMOS color sensor from Omnivision.
Supports output format: 10-bit Raw RGB.
The OV2680 has a single lane MIPI interface.

The driver exposes following V4L2 controls:
- auto/manual exposure,
- exposure,
- auto/manual gain,
- gain,
- horizontal/vertical flip,
- test pattern menu.
Supported resolution are only: QUXGA, 720P, UXGA.

Signed-off-by: Rui Miguel Silva <rui.si...@linaro.org>
---
 drivers/media/i2c/Kconfig  |   12 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov2680.c | 1094 
 3 files changed, 1107 insertions(+)
 create mode 100644 drivers/media/i2c/ov2680.c

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 9f18cd296841..39dc9f236ffa 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -586,6 +586,18 @@ config VIDEO_OV2659
  To compile this driver as a module, choose M here: the
  module will be called ov2659.
 
+config VIDEO_OV2680
+   tristate "OmniVision OV2680 sensor support"
+   depends on GPIOLIB && VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API
+   depends on MEDIA_CAMERA_SUPPORT
+   select V4L2_FWNODE
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the OmniVision
+ OV2680 camera sensor with a MIPI CSI-2 interface.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ov2680.
+
 config VIDEO_OV5640
tristate "OmniVision OV5640 sensor support"
depends on OF
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index c0f94cd8d56d..d0aba4d37b8d 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o
 obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
 obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
 obj-$(CONFIG_VIDEO_OV2640) += ov2640.o
+obj-$(CONFIG_VIDEO_OV2680) += ov2680.o
 obj-$(CONFIG_VIDEO_OV5640) += ov5640.o
 obj-$(CONFIG_VIDEO_OV5645) += ov5645.o
 obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c
new file mode 100644
index ..78f2be27a8f5
--- /dev/null
+++ b/drivers/media/i2c/ov2680.c
@@ -0,0 +1,1094 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Omnivision OV2680 CMOS Image Sensor driver
+ *
+ * Copyright (C) 2018 Linaro Ltd
+ *
+ * Based on OV5640 Sensor Driver
+ * Copyright (C) 2011-2013 Freescale Semiconductor, Inc. All Rights Reserved.
+ * Copyright (C) 2014-2017 Mentor Graphics Inc.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#define OV2680_XVCLK_MIN   600
+#define OV2680_XVCLK_MAX   2400
+
+#define OV2680_CHIP_ID 0x2680
+
+#define OV2680_REG_STREAM_CTRL 0x0100
+#define OV2680_REG_SOFT_RESET  0x0103
+
+#define OV2680_REG_CHIP_ID_HIGH0x300a
+#define OV2680_REG_CHIP_ID_LOW 0x300b
+
+#define OV2680_REG_R_MANUAL0x3503
+#define OV2680_REG_GAIN_PK 0x350a
+#define OV2680_REG_EXPOSURE_PK_HIGH0x3500
+#define OV2680_REG_TIMING_HTS  0x380c
+#define OV2680_REG_TIMING_VTS  0x380e
+#define OV2680_REG_FORMAT1 0x3820
+#define OV2680_REG_FORMAT2 0x3821
+
+#define OV2680_REG_ISP_CTRL00  0x5080
+
+#define OV2680_FRAME_RATE  30
+
+#define OV2680_REG_VALUE_8BIT  1
+#define OV2680_REG_VALUE_16BIT 2
+#define OV2680_REG_VALUE_24BIT 3
+
+enum ov2680_mode_id {
+   OV2680_MODE_QUXGA_800_600,
+   OV2680_MODE_720P_1280_720,
+   OV2680_MODE_UXGA_1600_1200,
+   OV2680_MODE_MAX,
+};
+
+struct reg_value {
+   u16 reg_addr;
+   u8 val;
+};
+
+struct ov2680_mode_info {
+   const char *name;
+   enum ov2680_mode_id id;
+   u32 width;
+   u32 height;
+   const struct reg_value *reg_data;
+   u32 reg_data_size;
+};
+
+struct ov2680_ctrls {
+   struct v4l2_ctrl_handler handler;
+   struct {
+   struct v4l2_ctrl *auto_exp;
+   struct v4l2_ctrl *exposure;
+   };
+   struct {
+   struct v4l2_ctrl *auto_gain;
+   struct v4l2_ctrl *gain;
+   };
+
+   struct v4l2_ctrl *hflip;
+   struct v4l2_ctrl *vflip;
+   struct v4l2_ctrl *test_pattern;
+};
+
+struct ov2680_dev {
+   struct i2c_client   *i2c_client;
+   struct v4l2_subdev  sd;
+
+   struct media_padpad;
+   struct clk  *xvclk;
+   u32 xvclk_freq;
+
+   struct gpio_desc*pwdn_gpio;
+   struct mutexlock; /* protect members */
+
+   boolmode_pending_changes;
+   b

[PATCH v2 1/2] media: ov2680: dt: Add bindings for OV2680

[Rui Miguel Silva]

Add device tree binding documentation for the OV2680 camera sensor.

CC: devicet...@vger.kernel.org
Signed-off-by: Rui Miguel Silva 
---
 .../devicetree/bindings/media/i2c/ov2680.txt   | 40 ++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/ov2680.txt 
b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
new file mode 100644
index ..0e29f1a113c0
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
@@ -0,0 +1,40 @@
+* Omnivision OV2680 MIPI CSI-2 sensor
+
+Required Properties:
+- compatible: should be "ovti,ov2680".
+- clocks: reference to the xvclk input clock.
+- clock-names: should be "xvclk".
+
+Optional Properties:
+- powerdown-gpios: reference to the GPIO connected to the powerdown pin,
+if any. This is an active high signal to the OV2680.
+
+The device node must contain one 'port' child node for its digital output
+video port, and this port must have a single endpoint in accordance with
+ the video interface bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Endpoint node required properties for CSI-2 connection are:
+- remote-endpoint: a phandle to the bus receiver's endpoint node.
+- clock-lanes: should be set to <0> (clock lane on hardware lane 0).
+- data-lanes: should be set to <1> (one CSI-2 lane supported).
+ 
+Example:
+
+ {
+   ov2680: camera-sensor@36 {
+   compatible = "ovti,ov2680";
+   reg = <0x36>;
+   clocks = <>;
+   clock-names = "xvclk";
+   powerdown-gpios = < 3 GPIO_ACTIVE_HIGH>;
+
+   port {
+   ov2680_mipi_ep: endpoint {
+   remote-endpoint = <_sensor_ep>;
+   clock-lanes = <0>;
+   data-lanes = <1>;
+   };
+   };
+   };
+};
-- 
2.16.2

[PATCH v2 2/2] media: ov2680: Add Omnivision OV2680 sensor driver

[Rui Miguel Silva]

This patch adds V4L2 sub-device driver for OV2680 image sensor.
The OV2680 is a 1/5" CMOS color sensor from Omnivision.
Supports output format: 10-bit Raw RGB.
The OV2680 has a single lane MIPI interface.

The driver exposes following V4L2 controls:
- auto/manual exposure,
- exposure,
- auto/manual gain,
- gain,
- horizontal/vertical flip,
- test pattern menu.
Supported resolution are only: QUXGA, 720P, UXGA.

Signed-off-by: Rui Miguel Silva 
---
 drivers/media/i2c/Kconfig  |   12 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov2680.c | 1094 
 3 files changed, 1107 insertions(+)
 create mode 100644 drivers/media/i2c/ov2680.c

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 9f18cd296841..39dc9f236ffa 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -586,6 +586,18 @@ config VIDEO_OV2659
  To compile this driver as a module, choose M here: the
  module will be called ov2659.
 
+config VIDEO_OV2680
+   tristate "OmniVision OV2680 sensor support"
+   depends on GPIOLIB && VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API
+   depends on MEDIA_CAMERA_SUPPORT
+   select V4L2_FWNODE
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the OmniVision
+ OV2680 camera sensor with a MIPI CSI-2 interface.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ov2680.
+
 config VIDEO_OV5640
tristate "OmniVision OV5640 sensor support"
depends on OF
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index c0f94cd8d56d..d0aba4d37b8d 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o
 obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
 obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
 obj-$(CONFIG_VIDEO_OV2640) += ov2640.o
+obj-$(CONFIG_VIDEO_OV2680) += ov2680.o
 obj-$(CONFIG_VIDEO_OV5640) += ov5640.o
 obj-$(CONFIG_VIDEO_OV5645) += ov5645.o
 obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c
new file mode 100644
index ..78f2be27a8f5
--- /dev/null
+++ b/drivers/media/i2c/ov2680.c
@@ -0,0 +1,1094 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Omnivision OV2680 CMOS Image Sensor driver
+ *
+ * Copyright (C) 2018 Linaro Ltd
+ *
+ * Based on OV5640 Sensor Driver
+ * Copyright (C) 2011-2013 Freescale Semiconductor, Inc. All Rights Reserved.
+ * Copyright (C) 2014-2017 Mentor Graphics Inc.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#define OV2680_XVCLK_MIN   600
+#define OV2680_XVCLK_MAX   2400
+
+#define OV2680_CHIP_ID 0x2680
+
+#define OV2680_REG_STREAM_CTRL 0x0100
+#define OV2680_REG_SOFT_RESET  0x0103
+
+#define OV2680_REG_CHIP_ID_HIGH0x300a
+#define OV2680_REG_CHIP_ID_LOW 0x300b
+
+#define OV2680_REG_R_MANUAL0x3503
+#define OV2680_REG_GAIN_PK 0x350a
+#define OV2680_REG_EXPOSURE_PK_HIGH0x3500
+#define OV2680_REG_TIMING_HTS  0x380c
+#define OV2680_REG_TIMING_VTS  0x380e
+#define OV2680_REG_FORMAT1 0x3820
+#define OV2680_REG_FORMAT2 0x3821
+
+#define OV2680_REG_ISP_CTRL00  0x5080
+
+#define OV2680_FRAME_RATE  30
+
+#define OV2680_REG_VALUE_8BIT  1
+#define OV2680_REG_VALUE_16BIT 2
+#define OV2680_REG_VALUE_24BIT 3
+
+enum ov2680_mode_id {
+   OV2680_MODE_QUXGA_800_600,
+   OV2680_MODE_720P_1280_720,
+   OV2680_MODE_UXGA_1600_1200,
+   OV2680_MODE_MAX,
+};
+
+struct reg_value {
+   u16 reg_addr;
+   u8 val;
+};
+
+struct ov2680_mode_info {
+   const char *name;
+   enum ov2680_mode_id id;
+   u32 width;
+   u32 height;
+   const struct reg_value *reg_data;
+   u32 reg_data_size;
+};
+
+struct ov2680_ctrls {
+   struct v4l2_ctrl_handler handler;
+   struct {
+   struct v4l2_ctrl *auto_exp;
+   struct v4l2_ctrl *exposure;
+   };
+   struct {
+   struct v4l2_ctrl *auto_gain;
+   struct v4l2_ctrl *gain;
+   };
+
+   struct v4l2_ctrl *hflip;
+   struct v4l2_ctrl *vflip;
+   struct v4l2_ctrl *test_pattern;
+};
+
+struct ov2680_dev {
+   struct i2c_client   *i2c_client;
+   struct v4l2_subdev  sd;
+
+   struct media_padpad;
+   struct clk  *xvclk;
+   u32 xvclk_freq;
+
+   struct gpio_desc*pwdn_gpio;
+   struct mutexlock; /* protect members */
+
+   boolmode_pending_changes;
+   boolis_enabled;
+  

[PATCH v2 0/2] media: Introduce Omnivision OV2680 driver

[Rui Miguel Silva]

Add driver and bindings for the OV2680 2 megapixel CMOS 1/5" sensor, which has
a single MIPI lane interface and output format of 10-bit Raw RGB.

Features supported are described in PATCH 2/2.

v1->v2:
Fabio Estevam:
- s/OV5640/OV2680 in PATCH 1/2 changelog

Sakari Ailus:
- add description on endpoint properties in bindings
- add single endpoint in bindings
- drop OF dependency
- cleanup includes
- fix case in Color Bars
- remove frame rate selection
- 8/16/24 bit register access in the same transaction
- merge _reset and _soft_reset to _enable and rename it to power_on 
- _gain_set use only the gain value (drop & 0x7ff)
- _gain_get remove the (0x377)
- single write/read at _exposure_set/get use write_reg24/read_reg24
- move mode_set_direct to _mode_set
- _mode_set set auto exposure/gain based on ctrl value
- s_frame_interval equal to g_frame_interval
- use closest match from: v4l: common: Add a function to obtain best size 
from a list 
- check v4l2_ctrl_new_std return in _init

- fix gain manual value in auto_cluster

Cheers,
    Rui

Rui Miguel Silva (2):
  media: ov2680: dt: Add bindings for OV2680
  media: ov2680: Add Omnivision OV2680 sensor driver

 .../devicetree/bindings/media/i2c/ov2680.txt   |   40 +
 drivers/media/i2c/Kconfig  |   12 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov2680.c | 1094 
 4 files changed, 1147 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt
 create mode 100644 drivers/media/i2c/ov2680.c

-- 
2.16.2

[PATCH v2 0/2] media: Introduce Omnivision OV2680 driver

[Rui Miguel Silva]

Add driver and bindings for the OV2680 2 megapixel CMOS 1/5" sensor, which has
a single MIPI lane interface and output format of 10-bit Raw RGB.

Features supported are described in PATCH 2/2.

v1->v2:
Fabio Estevam:
- s/OV5640/OV2680 in PATCH 1/2 changelog

Sakari Ailus:
- add description on endpoint properties in bindings
- add single endpoint in bindings
- drop OF dependency
- cleanup includes
- fix case in Color Bars
- remove frame rate selection
- 8/16/24 bit register access in the same transaction
- merge _reset and _soft_reset to _enable and rename it to power_on 
- _gain_set use only the gain value (drop & 0x7ff)
- _gain_get remove the (0x377)
- single write/read at _exposure_set/get use write_reg24/read_reg24
- move mode_set_direct to _mode_set
- _mode_set set auto exposure/gain based on ctrl value
- s_frame_interval equal to g_frame_interval
- use closest match from: v4l: common: Add a function to obtain best size 
from a list 
- check v4l2_ctrl_new_std return in _init

- fix gain manual value in auto_cluster

Cheers,
    Rui

Rui Miguel Silva (2):
  media: ov2680: dt: Add bindings for OV2680
  media: ov2680: Add Omnivision OV2680 sensor driver

 .../devicetree/bindings/media/i2c/ov2680.txt   |   40 +
 drivers/media/i2c/Kconfig  |   12 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov2680.c | 1094 
 4 files changed, 1147 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt
 create mode 100644 drivers/media/i2c/ov2680.c

-- 
2.16.2

Re: [PATCH 1/2] media: ov2680: dt: Add bindings for OV2680

[Rui Miguel Silva]

Hi Fabio,
On Thu 22 Feb 2018 at 11:31, Fabio Estevam wrote:
On Thu, Feb 22, 2018 at 7:23 AM, Rui Miguel Silva 
<rmf...@gmail.com> wrote:
Add device tree binding documentation for the OV5640 camera 
sensor.


s/OV5640/OV2680


Thanks for notice this ;).

---
Cheers,
Rui

Re: [PATCH 1/2] media: ov2680: dt: Add bindings for OV2680

[Rui Miguel Silva]

Hi Fabio,
On Thu 22 Feb 2018 at 11:31, Fabio Estevam wrote:
On Thu, Feb 22, 2018 at 7:23 AM, Rui Miguel Silva 
 wrote:
Add device tree binding documentation for the OV5640 camera 
sensor.


s/OV5640/OV2680


Thanks for notice this ;).

---
Cheers,
Rui

Re: [PATCH 1/2] media: ov2680: dt: Add bindings for OV2680

[Rui Miguel Silva]

Hi Sakari,
Thanks for the review.

On Thu 22 Feb 2018 at 10:59, Sakari Ailus wrote:

Hi Rui,

Thanks for the patchset.

Could you use "dt: bindings: " prefix in the subject?


Sure, no problem.



On Thu, Feb 22, 2018 at 10:23:37AM +, Rui Miguel Silva 
wrote:
Add device tree binding documentation for the OV5640 camera 
sensor.


CC: devicet...@vger.kernel.org
Signed-off-by: Rui Miguel Silva <rui.si...@linaro.org>
---
 .../devicetree/bindings/media/i2c/ov2680.txt   | 34 
 ++

 1 file changed, 34 insertions(+)
 create mode 100644 
 Documentation/devicetree/bindings/media/i2c/ov2680.txt


diff --git 
a/Documentation/devicetree/bindings/media/i2c/ov2680.txt 
b/Documentation/devicetree/bindings/media/i2c/ov2680.txt

new file mode 100644
index ..f9dc63ce5044
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
@@ -0,0 +1,34 @@
+* Omnivision OV2680 MIPI CSI-2 sensor
+
+Required Properties:
+- compatible: should be "ovti,ov2680"
+- clocks: reference to the xvclk input clock.
+- clock-names: should be "xvclk".
+
+Optional Properties:
+- powerdown-gpios: reference to the GPIO connected to the 
powerdown pin,
+		 if any. This is an active high signal to the 
OV2680.

+
+The device node must contain one 'port' child node for its 
digital output


Please add that the port contains a single endpoint as well.


Ack.



+video port, in accordance with the video interface bindings 
defined in

+Documentation/devicetree/bindings/media/video-interfaces.txt.


Please list required and optional endpoint properties as well.


OK.

---
Cheers,
Rui




+
+Example:
+
+ {
+   ov2680: camera-sensor@36 {
+   compatible = "ovti,ov2680";
+   reg = <0x36>;
+   clocks = <>;
+   clock-names = "xvclk";
+   powerdown-gpios = < 3 GPIO_ACTIVE_HIGH>;
+
+   port {
+   ov2680_mipi_ep: endpoint {
+remote-endpoint = 
<_sensor_ep>;

+   clock-lanes = <0>;
+   data-lanes = <1>;
+   };
+   };
+   };
+};

Re: [PATCH 1/2] media: ov2680: dt: Add bindings for OV2680

[Rui Miguel Silva]

Hi Sakari,
Thanks for the review.

On Thu 22 Feb 2018 at 10:59, Sakari Ailus wrote:

Hi Rui,

Thanks for the patchset.

Could you use "dt: bindings: " prefix in the subject?


Sure, no problem.



On Thu, Feb 22, 2018 at 10:23:37AM +, Rui Miguel Silva 
wrote:
Add device tree binding documentation for the OV5640 camera 
sensor.


CC: devicet...@vger.kernel.org
Signed-off-by: Rui Miguel Silva 
---
 .../devicetree/bindings/media/i2c/ov2680.txt   | 34 
 ++

 1 file changed, 34 insertions(+)
 create mode 100644 
 Documentation/devicetree/bindings/media/i2c/ov2680.txt


diff --git 
a/Documentation/devicetree/bindings/media/i2c/ov2680.txt 
b/Documentation/devicetree/bindings/media/i2c/ov2680.txt

new file mode 100644
index ..f9dc63ce5044
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
@@ -0,0 +1,34 @@
+* Omnivision OV2680 MIPI CSI-2 sensor
+
+Required Properties:
+- compatible: should be "ovti,ov2680"
+- clocks: reference to the xvclk input clock.
+- clock-names: should be "xvclk".
+
+Optional Properties:
+- powerdown-gpios: reference to the GPIO connected to the 
powerdown pin,
+		 if any. This is an active high signal to the 
OV2680.

+
+The device node must contain one 'port' child node for its 
digital output


Please add that the port contains a single endpoint as well.


Ack.



+video port, in accordance with the video interface bindings 
defined in

+Documentation/devicetree/bindings/media/video-interfaces.txt.


Please list required and optional endpoint properties as well.


OK.

---
Cheers,
Rui




+
+Example:
+
+ {
+   ov2680: camera-sensor@36 {
+   compatible = "ovti,ov2680";
+   reg = <0x36>;
+   clocks = <>;
+   clock-names = "xvclk";
+   powerdown-gpios = < 3 GPIO_ACTIVE_HIGH>;
+
+   port {
+   ov2680_mipi_ep: endpoint {
+remote-endpoint = 
<_sensor_ep>;

+   clock-lanes = <0>;
+   data-lanes = <1>;
+   };
+   };
+   };
+};

Re: [PATCH 2/2] media: ov2680: Add Omnivision OV2680 sensor driver

[Rui Miguel Silva]

Hi Sakari,
Thanks for your review.

On Fri 23 Feb 2018 at 07:50, Sakari Ailus wrote:

Hi Rui,

On Thu, Feb 22, 2018 at 10:23:38AM +, Rui Miguel Silva 
wrote:

This patch adds V4L2 sub-device driver for OV2680 image sensor.
The OV2680 is a 1/5" CMOS color sensor from Omnivision.
Supports output format: 10-bit Raw RGB.
The OV2680 has a single lane MIPI interface.

The driver exposes following V4L2 controls:
- auto/manual exposure,
- exposure,
- auto/manual gain,
- gain,
- horizontal/vertical flip,
- test pattern menu.
Supported resolution are only: QUXGA, 720P, UXGA.

Signed-off-by: Rui Miguel Silva <rui.si...@linaro.org>
---
 drivers/media/i2c/Kconfig  |   13 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov2680.c | 1189 
 

 3 files changed, 1203 insertions(+)
 create mode 100644 drivers/media/i2c/ov2680.c

diff --git a/drivers/media/i2c/Kconfig 
b/drivers/media/i2c/Kconfig

index 9f18cd296841..089103d29171 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -586,6 +586,19 @@ config VIDEO_OV2659
  To compile this driver as a module, choose M here: the
  module will be called ov2659.
 
+config VIDEO_OV2680

+   tristate "OmniVision OV2680 sensor support"
+   depends on OF


I think you can drop OF dependency here.


Agree.



+	depends on GPIOLIB && VIDEO_V4L2 && I2C && 
VIDEO_V4L2_SUBDEV_API

+   depends on MEDIA_CAMERA_SUPPORT
+   select V4L2_FWNODE
+   ---help---
+	  This is a Video4Linux2 sensor-level driver for the 
OmniVision

+ OV2680 camera sensor with a MIPI CSI-2 interface.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ov2680.
+
 config VIDEO_OV5640
tristate "OmniVision OV5640 sensor support"
depends on OF
diff --git a/drivers/media/i2c/Makefile 
b/drivers/media/i2c/Makefile

index c0f94cd8d56d..d0aba4d37b8d 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += 
sony-btf-mpx.o

 obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
 obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
 obj-$(CONFIG_VIDEO_OV2640) += ov2640.o
+obj-$(CONFIG_VIDEO_OV2680) += ov2680.o
 obj-$(CONFIG_VIDEO_OV5640) += ov5640.o
 obj-$(CONFIG_VIDEO_OV5645) += ov5645.o
 obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
diff --git a/drivers/media/i2c/ov2680.c 
b/drivers/media/i2c/ov2680.c

new file mode 100644
index ..64c1c2b03f97
--- /dev/null
+++ b/drivers/media/i2c/ov2680.c
@@ -0,0 +1,1189 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Omnivision OV2680 CMOS Image Sensor driver
+ *
+ * Copyright (C) 2018 Linaro Ltd
+ *
+ * Based on OV5640 Sensor Driver
+ * Copyright (C) 2011-2013 Freescale Semiconductor, Inc. All 
Rights Reserved.

+ * Copyright (C) 2014-2017 Mentor Graphics Inc.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 


Do you need of_gpio.h?


Yeah, I need only to include the gpio/consumer.h for the
devm_gpiod_get_optional call.




+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 


Do you need all of these? At least v4l2-event.h and 
v4l2-image-sizes.h seem

redundant.


Agree, will clean up this includes.




+
+#define OV2680_XVCLK_MIN   600
+#define OV2680_XVCLK_MAX   2400
+
+#define OV2680_CHIP_ID_HIGH0x26
+#define OV2680_CHIP_ID_LOW 0x80
+
+#define OV2680_REG_STREAM_CTRL 0x0100
+#define OV2680_REG_SOFT_RESET  0x0103
+
+#define OV2680_REG_CHIP_ID_HIGH0x300a
+#define OV2680_REG_CHIP_ID_LOW 0x300b
+
+#define OV2680_REG_R_MANUAL0x3503
+#define OV2680_REG_GAIN_PK 0x350a
+#define OV2680_REG_EXPOSURE_PK_HIGH0x3500
+#define OV2680_REG_EXPOSURE_PK_MED 0x3501
+#define OV2680_REG_EXPOSURE_PK_LOW 0x3502
+#define OV2680_REG_TIMING_HTS  0x380c
+#define OV2680_REG_TIMING_VTS  0x380e
+#define OV2680_REG_FORMAT1 0x3820
+#define OV2680_REG_FORMAT2 0x3821
+
+#define OV2680_REG_ISP_CTRL00  0x5080
+
+enum ov2680_frame_rate {
+   OV2680_30_FPS,
+   OV2680_FRAMERATES_MAX,
+};
+
+static const int ov2680_framerates[] = {
+   [OV2680_30_FPS] = 30,
+};
+
+enum ov2680_mode_id {
+   OV2680_MODE_QUXGA_800_600,
+   OV2680_MODE_720P_1280_720,
+   OV2680_MODE_UXGA_1600_1200,
+   OV2680_MODE_MAX,
+};
+
+struct reg_value {
+   u16 reg_addr;
+   u8 val;
+};
+
+struct ov2680_mode_info {
+   const char *name;
+   enum ov2680_mode_id id;
+   u32 width;
+   u32 height;
+   const struct reg_value *reg_data;
+   u32 reg_data_size;
+};
+
+struct ov2680_ctrls {
+   struct v4l2_ctrl_handler handler;
+   struct {
+   struct v4l2_ctrl *auto_exp;
+   struct v4l2_ctrl *exposure;
+   };
+   struct {
+   struct

Re: [PATCH 2/2] media: ov2680: Add Omnivision OV2680 sensor driver

[Rui Miguel Silva]

Hi Sakari,
Thanks for your review.

On Fri 23 Feb 2018 at 07:50, Sakari Ailus wrote:

Hi Rui,

On Thu, Feb 22, 2018 at 10:23:38AM +, Rui Miguel Silva 
wrote:

This patch adds V4L2 sub-device driver for OV2680 image sensor.
The OV2680 is a 1/5" CMOS color sensor from Omnivision.
Supports output format: 10-bit Raw RGB.
The OV2680 has a single lane MIPI interface.

The driver exposes following V4L2 controls:
- auto/manual exposure,
- exposure,
- auto/manual gain,
- gain,
- horizontal/vertical flip,
- test pattern menu.
Supported resolution are only: QUXGA, 720P, UXGA.

Signed-off-by: Rui Miguel Silva 
---
 drivers/media/i2c/Kconfig  |   13 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov2680.c | 1189 
 

 3 files changed, 1203 insertions(+)
 create mode 100644 drivers/media/i2c/ov2680.c

diff --git a/drivers/media/i2c/Kconfig 
b/drivers/media/i2c/Kconfig

index 9f18cd296841..089103d29171 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -586,6 +586,19 @@ config VIDEO_OV2659
  To compile this driver as a module, choose M here: the
  module will be called ov2659.
 
+config VIDEO_OV2680

+   tristate "OmniVision OV2680 sensor support"
+   depends on OF


I think you can drop OF dependency here.


Agree.



+	depends on GPIOLIB && VIDEO_V4L2 && I2C && 
VIDEO_V4L2_SUBDEV_API

+   depends on MEDIA_CAMERA_SUPPORT
+   select V4L2_FWNODE
+   ---help---
+	  This is a Video4Linux2 sensor-level driver for the 
OmniVision

+ OV2680 camera sensor with a MIPI CSI-2 interface.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ov2680.
+
 config VIDEO_OV5640
tristate "OmniVision OV5640 sensor support"
depends on OF
diff --git a/drivers/media/i2c/Makefile 
b/drivers/media/i2c/Makefile

index c0f94cd8d56d..d0aba4d37b8d 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += 
sony-btf-mpx.o

 obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
 obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
 obj-$(CONFIG_VIDEO_OV2640) += ov2640.o
+obj-$(CONFIG_VIDEO_OV2680) += ov2680.o
 obj-$(CONFIG_VIDEO_OV5640) += ov5640.o
 obj-$(CONFIG_VIDEO_OV5645) += ov5645.o
 obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
diff --git a/drivers/media/i2c/ov2680.c 
b/drivers/media/i2c/ov2680.c

new file mode 100644
index ..64c1c2b03f97
--- /dev/null
+++ b/drivers/media/i2c/ov2680.c
@@ -0,0 +1,1189 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Omnivision OV2680 CMOS Image Sensor driver
+ *
+ * Copyright (C) 2018 Linaro Ltd
+ *
+ * Based on OV5640 Sensor Driver
+ * Copyright (C) 2011-2013 Freescale Semiconductor, Inc. All 
Rights Reserved.

+ * Copyright (C) 2014-2017 Mentor Graphics Inc.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 


Do you need of_gpio.h?


Yeah, I need only to include the gpio/consumer.h for the
devm_gpiod_get_optional call.




+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 


Do you need all of these? At least v4l2-event.h and 
v4l2-image-sizes.h seem

redundant.


Agree, will clean up this includes.




+
+#define OV2680_XVCLK_MIN   600
+#define OV2680_XVCLK_MAX   2400
+
+#define OV2680_CHIP_ID_HIGH0x26
+#define OV2680_CHIP_ID_LOW 0x80
+
+#define OV2680_REG_STREAM_CTRL 0x0100
+#define OV2680_REG_SOFT_RESET  0x0103
+
+#define OV2680_REG_CHIP_ID_HIGH0x300a
+#define OV2680_REG_CHIP_ID_LOW 0x300b
+
+#define OV2680_REG_R_MANUAL0x3503
+#define OV2680_REG_GAIN_PK 0x350a
+#define OV2680_REG_EXPOSURE_PK_HIGH0x3500
+#define OV2680_REG_EXPOSURE_PK_MED 0x3501
+#define OV2680_REG_EXPOSURE_PK_LOW 0x3502
+#define OV2680_REG_TIMING_HTS  0x380c
+#define OV2680_REG_TIMING_VTS  0x380e
+#define OV2680_REG_FORMAT1 0x3820
+#define OV2680_REG_FORMAT2 0x3821
+
+#define OV2680_REG_ISP_CTRL00  0x5080
+
+enum ov2680_frame_rate {
+   OV2680_30_FPS,
+   OV2680_FRAMERATES_MAX,
+};
+
+static const int ov2680_framerates[] = {
+   [OV2680_30_FPS] = 30,
+};
+
+enum ov2680_mode_id {
+   OV2680_MODE_QUXGA_800_600,
+   OV2680_MODE_720P_1280_720,
+   OV2680_MODE_UXGA_1600_1200,
+   OV2680_MODE_MAX,
+};
+
+struct reg_value {
+   u16 reg_addr;
+   u8 val;
+};
+
+struct ov2680_mode_info {
+   const char *name;
+   enum ov2680_mode_id id;
+   u32 width;
+   u32 height;
+   const struct reg_value *reg_data;
+   u32 reg_data_size;
+};
+
+struct ov2680_ctrls {
+   struct v4l2_ctrl_handler handler;
+   struct {
+   struct v4l2_ctrl *auto_exp;
+   struct v4l2_ctrl *exposure;
+   };
+   struct {
+   struct v4l2_ctrl *

[PATCH v4 2/4] crypto: caam - do not use mem and emi_slow clock for imx7x

[Rui Miguel Silva]

I.MX7x only use two clocks for the CAAM module, so make sure we do not try to
use the mem and the emi_slow clock when running in that imx7d and imx7s machine
type.

Cc: "Horia Geantă" <horia.gea...@nxp.com>
Cc: Aymen Sghaier <aymen.sgha...@nxp.com>
Cc: Fabio Estevam <fabio.este...@nxp.com>
Cc: Peng Fan <peng@nxp.com>
Cc: "David S. Miller" <da...@davemloft.net>
Cc: Lukas Auer <lukas.a...@aisec.fraunhofer.de>
Signed-off-by: Rui Miguel Silva <rui.si...@linaro.org>
---
 drivers/crypto/caam/ctrl.c | 39 ---
 1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index 361e750f9cba..e4cc636e1104 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -337,7 +337,8 @@ static int caam_remove(struct platform_device *pdev)
 
/* shut clocks off before finalizing shutdown */
clk_disable_unprepare(ctrlpriv->caam_ipg);
-   clk_disable_unprepare(ctrlpriv->caam_mem);
+   if (ctrlpriv->caam_mem)
+   clk_disable_unprepare(ctrlpriv->caam_mem);
clk_disable_unprepare(ctrlpriv->caam_aclk);
if (ctrlpriv->caam_emi_slow)
clk_disable_unprepare(ctrlpriv->caam_emi_slow);
@@ -466,14 +467,17 @@ static int caam_probe(struct platform_device *pdev)
}
ctrlpriv->caam_ipg = clk;
 
-   clk = caam_drv_identify_clk(>dev, "mem");
-   if (IS_ERR(clk)) {
-   ret = PTR_ERR(clk);
-   dev_err(>dev,
-   "can't identify CAAM mem clk: %d\n", ret);
-   return ret;
+   if (!of_machine_is_compatible("fsl,imx7d") &&
+   !of_machine_is_compatible("fsl,imx7s")) {
+   clk = caam_drv_identify_clk(>dev, "mem");
+   if (IS_ERR(clk)) {
+   ret = PTR_ERR(clk);
+   dev_err(>dev,
+   "can't identify CAAM mem clk: %d\n", ret);
+   return ret;
+   }
+   ctrlpriv->caam_mem = clk;
}
-   ctrlpriv->caam_mem = clk;
 
clk = caam_drv_identify_clk(>dev, "aclk");
if (IS_ERR(clk)) {
@@ -484,7 +488,9 @@ static int caam_probe(struct platform_device *pdev)
}
ctrlpriv->caam_aclk = clk;
 
-   if (!of_machine_is_compatible("fsl,imx6ul")) {
+   if (!of_machine_is_compatible("fsl,imx6ul") &&
+   !of_machine_is_compatible("fsl,imx7d") &&
+   !of_machine_is_compatible("fsl,imx7s")) {
clk = caam_drv_identify_clk(>dev, "emi_slow");
if (IS_ERR(clk)) {
ret = PTR_ERR(clk);
@@ -501,11 +507,13 @@ static int caam_probe(struct platform_device *pdev)
return ret;
}
 
-   ret = clk_prepare_enable(ctrlpriv->caam_mem);
-   if (ret < 0) {
-   dev_err(>dev, "can't enable CAAM secure mem clock: %d\n",
-   ret);
-   goto disable_caam_ipg;
+   if (ctrlpriv->caam_mem) {
+   ret = clk_prepare_enable(ctrlpriv->caam_mem);
+   if (ret < 0) {
+   dev_err(>dev, "can't enable CAAM secure mem 
clock: %d\n",
+   ret);
+   goto disable_caam_ipg;
+   }
}
 
ret = clk_prepare_enable(ctrlpriv->caam_aclk);
@@ -826,7 +834,8 @@ static int caam_probe(struct platform_device *pdev)
 disable_caam_aclk:
clk_disable_unprepare(ctrlpriv->caam_aclk);
 disable_caam_mem:
-   clk_disable_unprepare(ctrlpriv->caam_mem);
+   if (ctrlpriv->caam_mem)
+   clk_disable_unprepare(ctrlpriv->caam_mem);
 disable_caam_ipg:
clk_disable_unprepare(ctrlpriv->caam_ipg);
return ret;
-- 
2.16.2

[PATCH v4 2/4] crypto: caam - do not use mem and emi_slow clock for imx7x

[Rui Miguel Silva]

I.MX7x only use two clocks for the CAAM module, so make sure we do not try to
use the mem and the emi_slow clock when running in that imx7d and imx7s machine
type.

Cc: "Horia Geantă" 
Cc: Aymen Sghaier 
Cc: Fabio Estevam 
Cc: Peng Fan 
Cc: "David S. Miller" 
Cc: Lukas Auer 
Signed-off-by: Rui Miguel Silva 
---
 drivers/crypto/caam/ctrl.c | 39 ---
 1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index 361e750f9cba..e4cc636e1104 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -337,7 +337,8 @@ static int caam_remove(struct platform_device *pdev)
 
/* shut clocks off before finalizing shutdown */
clk_disable_unprepare(ctrlpriv->caam_ipg);
-   clk_disable_unprepare(ctrlpriv->caam_mem);
+   if (ctrlpriv->caam_mem)
+   clk_disable_unprepare(ctrlpriv->caam_mem);
clk_disable_unprepare(ctrlpriv->caam_aclk);
if (ctrlpriv->caam_emi_slow)
clk_disable_unprepare(ctrlpriv->caam_emi_slow);
@@ -466,14 +467,17 @@ static int caam_probe(struct platform_device *pdev)
}
ctrlpriv->caam_ipg = clk;
 
-   clk = caam_drv_identify_clk(>dev, "mem");
-   if (IS_ERR(clk)) {
-   ret = PTR_ERR(clk);
-   dev_err(>dev,
-   "can't identify CAAM mem clk: %d\n", ret);
-   return ret;
+   if (!of_machine_is_compatible("fsl,imx7d") &&
+   !of_machine_is_compatible("fsl,imx7s")) {
+   clk = caam_drv_identify_clk(>dev, "mem");
+   if (IS_ERR(clk)) {
+   ret = PTR_ERR(clk);
+   dev_err(>dev,
+   "can't identify CAAM mem clk: %d\n", ret);
+   return ret;
+   }
+   ctrlpriv->caam_mem = clk;
}
-   ctrlpriv->caam_mem = clk;
 
clk = caam_drv_identify_clk(>dev, "aclk");
if (IS_ERR(clk)) {
@@ -484,7 +488,9 @@ static int caam_probe(struct platform_device *pdev)
}
ctrlpriv->caam_aclk = clk;
 
-   if (!of_machine_is_compatible("fsl,imx6ul")) {
+   if (!of_machine_is_compatible("fsl,imx6ul") &&
+   !of_machine_is_compatible("fsl,imx7d") &&
+   !of_machine_is_compatible("fsl,imx7s")) {
clk = caam_drv_identify_clk(>dev, "emi_slow");
if (IS_ERR(clk)) {
ret = PTR_ERR(clk);
@@ -501,11 +507,13 @@ static int caam_probe(struct platform_device *pdev)
return ret;
}
 
-   ret = clk_prepare_enable(ctrlpriv->caam_mem);
-   if (ret < 0) {
-   dev_err(>dev, "can't enable CAAM secure mem clock: %d\n",
-   ret);
-   goto disable_caam_ipg;
+   if (ctrlpriv->caam_mem) {
+   ret = clk_prepare_enable(ctrlpriv->caam_mem);
+   if (ret < 0) {
+   dev_err(>dev, "can't enable CAAM secure mem 
clock: %d\n",
+   ret);
+   goto disable_caam_ipg;
+   }
}
 
ret = clk_prepare_enable(ctrlpriv->caam_aclk);
@@ -826,7 +834,8 @@ static int caam_probe(struct platform_device *pdev)
 disable_caam_aclk:
clk_disable_unprepare(ctrlpriv->caam_aclk);
 disable_caam_mem:
-   clk_disable_unprepare(ctrlpriv->caam_mem);
+   if (ctrlpriv->caam_mem)
+   clk_disable_unprepare(ctrlpriv->caam_mem);
 disable_caam_ipg:
clk_disable_unprepare(ctrlpriv->caam_ipg);
return ret;
-- 
2.16.2

[PATCH v4 1/4] crypto: caam - Fix null dereference at error path

[Rui Miguel Silva]

caam_remove already removes the debugfs entry, so we need to remove the one
immediately before calling caam_remove.

This fix a NULL dereference at error paths is caam_probe fail.

Fixes: 67c2315def06 ("crypto: caam - add Queue Interface (QI) backend support")

Tested-by: Ryan Harkin <ryan.har...@linaro.org>
Cc: "Horia Geantă" <horia.gea...@nxp.com>
Cc: Aymen Sghaier <aymen.sgha...@nxp.com>
Cc: Fabio Estevam <fabio.este...@nxp.com>
Cc: Peng Fan <peng@nxp.com>
Cc: "David S. Miller" <da...@davemloft.net>
Cc: Lukas Auer <lukas.a...@aisec.fraunhofer.de>
Cc: <sta...@vger.kernel.org> # 4.12+
Reviewed-by: Horia Geantă <horia.gea...@nxp.com>
Signed-off-by: Rui Miguel Silva <rui.si...@linaro.org>
---
 drivers/crypto/caam/ctrl.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index e843cf410373..361e750f9cba 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -815,9 +815,6 @@ static int caam_probe(struct platform_device *pdev)
return 0;
 
 caam_remove:
-#ifdef CONFIG_DEBUG_FS
-   debugfs_remove_recursive(ctrlpriv->dfs_root);
-#endif
caam_remove(pdev);
return ret;
 
-- 
2.16.2

[PATCH v4 1/4] crypto: caam - Fix null dereference at error path

[Rui Miguel Silva]

caam_remove already removes the debugfs entry, so we need to remove the one
immediately before calling caam_remove.

This fix a NULL dereference at error paths is caam_probe fail.

Fixes: 67c2315def06 ("crypto: caam - add Queue Interface (QI) backend support")

Tested-by: Ryan Harkin 
Cc: "Horia Geantă" 
Cc: Aymen Sghaier 
Cc: Fabio Estevam 
Cc: Peng Fan 
Cc: "David S. Miller" 
Cc: Lukas Auer 
Cc:  # 4.12+
Reviewed-by: Horia Geantă 
Signed-off-by: Rui Miguel Silva 
---
 drivers/crypto/caam/ctrl.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index e843cf410373..361e750f9cba 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -815,9 +815,6 @@ static int caam_probe(struct platform_device *pdev)
return 0;
 
 caam_remove:
-#ifdef CONFIG_DEBUG_FS
-   debugfs_remove_recursive(ctrlpriv->dfs_root);
-#endif
caam_remove(pdev);
return ret;
 
-- 
2.16.2

[PATCH v4 3/4] clk: imx7d: add CAAM clock

[Rui Miguel Silva]

Add CAAM clock so that we could use the Cryptographic Acceleration and
Assurance Module (CAAM) hardware block.

Cc: Michael Turquette <mturque...@baylibre.com>
Cc: Stephen Boyd <sb...@codeaurora.org>
Cc: linux-...@vger.kernel.org
Cc: "Horia Geantă" <horia.gea...@nxp.com>
Cc: Aymen Sghaier <aymen.sgha...@nxp.com>
Cc: Fabio Estevam <fabio.este...@nxp.com>
Cc: Peng Fan <peng@nxp.com>
Cc: "David S. Miller" <da...@davemloft.net>
Cc: Lukas Auer <lukas.a...@aisec.fraunhofer.de>
Reviewed-by: Fabio Estevam <fabio.este...@nxp.com>
Signed-off-by: Rui Miguel Silva <rui.si...@linaro.org>
---
 drivers/clk/imx/clk-imx7d.c | 1 +
 include/dt-bindings/clock/imx7d-clock.h | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/imx/clk-imx7d.c b/drivers/clk/imx/clk-imx7d.c
index 80dc211eb74b..52ab096fb39c 100644
--- a/drivers/clk/imx/clk-imx7d.c
+++ b/drivers/clk/imx/clk-imx7d.c
@@ -795,6 +795,7 @@ static void __init imx7d_clocks_init(struct device_node 
*ccm_node)
clks[IMX7D_DRAM_PHYM_ALT_ROOT_CLK] = 
imx_clk_gate4("dram_phym_alt_root_clk", "dram_phym_alt_post_div", base + 
0x4130, 0);
clks[IMX7D_DRAM_ALT_ROOT_CLK] = imx_clk_gate4("dram_alt_root_clk", 
"dram_alt_post_div", base + 0x4130, 0);
clks[IMX7D_OCOTP_CLK] = imx_clk_gate4("ocotp_clk", "ipg_root_clk", base 
+ 0x4230, 0);
+   clks[IMX7D_CAAM_CLK] = imx_clk_gate4("caam_clk", "ipg_root_clk", base + 
0x4240, 0);
clks[IMX7D_USB_HSIC_ROOT_CLK] = imx_clk_gate4("usb_hsic_root_clk", 
"usb_hsic_post_div", base + 0x4420, 0);
clks[IMX7D_SDMA_CORE_CLK] = imx_clk_gate4("sdma_root_clk", 
"ahb_root_clk", base + 0x4480, 0);
clks[IMX7D_PCIE_CTRL_ROOT_CLK] = imx_clk_gate4("pcie_ctrl_root_clk", 
"pcie_ctrl_post_div", base + 0x4600, 0);
diff --git a/include/dt-bindings/clock/imx7d-clock.h 
b/include/dt-bindings/clock/imx7d-clock.h
index e2f99ae72d5c..2bc5618feaeb 100644
--- a/include/dt-bindings/clock/imx7d-clock.h
+++ b/include/dt-bindings/clock/imx7d-clock.h
@@ -452,5 +452,6 @@
 #define IMX7D_OCOTP_CLK439
 #define IMX7D_NAND_RAWNAND_CLK 440
 #define IMX7D_NAND_USDHC_BUS_RAWNAND_CLK 441
-#define IMX7D_CLK_END  442
+#define IMX7D_CAAM_CLK 442
+#define IMX7D_CLK_END  443
 #endif /* __DT_BINDINGS_CLOCK_IMX7D_H */
-- 
2.16.2

[PATCH v4 3/4] clk: imx7d: add CAAM clock

[Rui Miguel Silva]

Add CAAM clock so that we could use the Cryptographic Acceleration and
Assurance Module (CAAM) hardware block.

Cc: Michael Turquette 
Cc: Stephen Boyd 
Cc: linux-...@vger.kernel.org
Cc: "Horia Geantă" 
Cc: Aymen Sghaier 
Cc: Fabio Estevam 
Cc: Peng Fan 
Cc: "David S. Miller" 
Cc: Lukas Auer 
Reviewed-by: Fabio Estevam 
Signed-off-by: Rui Miguel Silva 
---
 drivers/clk/imx/clk-imx7d.c | 1 +
 include/dt-bindings/clock/imx7d-clock.h | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/imx/clk-imx7d.c b/drivers/clk/imx/clk-imx7d.c
index 80dc211eb74b..52ab096fb39c 100644
--- a/drivers/clk/imx/clk-imx7d.c
+++ b/drivers/clk/imx/clk-imx7d.c
@@ -795,6 +795,7 @@ static void __init imx7d_clocks_init(struct device_node 
*ccm_node)
clks[IMX7D_DRAM_PHYM_ALT_ROOT_CLK] = 
imx_clk_gate4("dram_phym_alt_root_clk", "dram_phym_alt_post_div", base + 
0x4130, 0);
clks[IMX7D_DRAM_ALT_ROOT_CLK] = imx_clk_gate4("dram_alt_root_clk", 
"dram_alt_post_div", base + 0x4130, 0);
clks[IMX7D_OCOTP_CLK] = imx_clk_gate4("ocotp_clk", "ipg_root_clk", base 
+ 0x4230, 0);
+   clks[IMX7D_CAAM_CLK] = imx_clk_gate4("caam_clk", "ipg_root_clk", base + 
0x4240, 0);
clks[IMX7D_USB_HSIC_ROOT_CLK] = imx_clk_gate4("usb_hsic_root_clk", 
"usb_hsic_post_div", base + 0x4420, 0);
clks[IMX7D_SDMA_CORE_CLK] = imx_clk_gate4("sdma_root_clk", 
"ahb_root_clk", base + 0x4480, 0);
clks[IMX7D_PCIE_CTRL_ROOT_CLK] = imx_clk_gate4("pcie_ctrl_root_clk", 
"pcie_ctrl_post_div", base + 0x4600, 0);
diff --git a/include/dt-bindings/clock/imx7d-clock.h 
b/include/dt-bindings/clock/imx7d-clock.h
index e2f99ae72d5c..2bc5618feaeb 100644
--- a/include/dt-bindings/clock/imx7d-clock.h
+++ b/include/dt-bindings/clock/imx7d-clock.h
@@ -452,5 +452,6 @@
 #define IMX7D_OCOTP_CLK439
 #define IMX7D_NAND_RAWNAND_CLK 440
 #define IMX7D_NAND_USDHC_BUS_RAWNAND_CLK 441
-#define IMX7D_CLK_END  442
+#define IMX7D_CAAM_CLK 442
+#define IMX7D_CLK_END  443
 #endif /* __DT_BINDINGS_CLOCK_IMX7D_H */
-- 
2.16.2

[PATCH v4 4/4] ARM: dts: imx7s: add CAAM device node

[Rui Miguel Silva]

Add CAAM device node to the i.MX7s device tree.

Cc: Shawn Guo <shawn...@kernel.org>
Cc: Sascha Hauer <ker...@pengutronix.de>
Cc: devicet...@vger.kernel.org
Cc: "Horia Geantă" <horia.gea...@nxp.com>
Cc: Aymen Sghaier <aymen.sgha...@nxp.com>
Cc: Fabio Estevam <fabio.este...@nxp.com>
Cc: Peng Fan <peng@nxp.com>
Cc: "David S. Miller" <da...@davemloft.net>
Cc: Lukas Auer <lukas.a...@aisec.fraunhofer.de>
Signed-off-by: Rui Miguel Silva <rui.si...@linaro.org>
---
 arch/arm/boot/dts/imx7s.dtsi | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index 9aa2bb998552..d88c179e4c00 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -822,6 +822,36 @@
status = "disabled";
};
 
+   crypto: caam@3090 {
+   compatible = "fsl,sec-v4.0";
+   #address-cells = <1>;
+   #size-cells = <1>;
+   reg = <0x3090 0x4>;
+   ranges = <0 0x3090 0x4>;
+   interrupts = ;
+   clocks = < IMX7D_CAAM_CLK>,
+< IMX7D_AHB_CHANNEL_ROOT_CLK>;
+   clock-names = "ipg", "aclk";
+
+   sec_jr0: jr0@1000 {
+   compatible = "fsl,sec-v4.0-job-ring";
+   reg = <0x1000 0x1000>;
+   interrupts = ;
+   };
+
+   sec_jr1: jr1@2000 {
+   compatible = "fsl,sec-v4.0-job-ring";
+   reg = <0x2000 0x1000>;
+   interrupts = ;
+   };
+
+   sec_jr2: jr1@3000 {
+   compatible = "fsl,sec-v4.0-job-ring";
+   reg = <0x3000 0x1000>;
+   interrupts = ;
+   };
+   };
+
flexcan1: can@30a0 {
compatible = "fsl,imx7d-flexcan", 
"fsl,imx6q-flexcan";
reg = <0x30a0 0x1>;
-- 
2.16.2

[PATCH v4 4/4] ARM: dts: imx7s: add CAAM device node

[Rui Miguel Silva]

Add CAAM device node to the i.MX7s device tree.

Cc: Shawn Guo 
Cc: Sascha Hauer 
Cc: devicet...@vger.kernel.org
Cc: "Horia Geantă" 
Cc: Aymen Sghaier 
Cc: Fabio Estevam 
Cc: Peng Fan 
Cc: "David S. Miller" 
Cc: Lukas Auer 
Signed-off-by: Rui Miguel Silva 
---
 arch/arm/boot/dts/imx7s.dtsi | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/arch/arm/boot/dts/imx7s.dtsi b/arch/arm/boot/dts/imx7s.dtsi
index 9aa2bb998552..d88c179e4c00 100644
--- a/arch/arm/boot/dts/imx7s.dtsi
+++ b/arch/arm/boot/dts/imx7s.dtsi
@@ -822,6 +822,36 @@
status = "disabled";
};
 
+   crypto: caam@3090 {
+   compatible = "fsl,sec-v4.0";
+   #address-cells = <1>;
+   #size-cells = <1>;
+   reg = <0x3090 0x4>;
+   ranges = <0 0x3090 0x4>;
+   interrupts = ;
+   clocks = < IMX7D_CAAM_CLK>,
+< IMX7D_AHB_CHANNEL_ROOT_CLK>;
+   clock-names = "ipg", "aclk";
+
+   sec_jr0: jr0@1000 {
+   compatible = "fsl,sec-v4.0-job-ring";
+   reg = <0x1000 0x1000>;
+   interrupts = ;
+   };
+
+   sec_jr1: jr1@2000 {
+   compatible = "fsl,sec-v4.0-job-ring";
+   reg = <0x2000 0x1000>;
+   interrupts = ;
+   };
+
+   sec_jr2: jr1@3000 {
+   compatible = "fsl,sec-v4.0-job-ring";
+   reg = <0x3000 0x1000>;
+   interrupts = ;
+   };
+   };
+
flexcan1: can@30a0 {
compatible = "fsl,imx7d-flexcan", 
"fsl,imx6q-flexcan";
reg = <0x30a0 0x1>;
-- 
2.16.2

[PATCH v4 0/4] Enable CAAM on i.MX7s fix TrustZone issues

[Rui Miguel Silva]

For v4 I am taking over this patch series as all the left over patches were
implemented by me.

V4:
- removed patch: [PATCH v3 2/5] crypto: caam: Fix endless loop when RNG is
already initialized from the series since Horia presented a better fix for the
 endless loop in case of fail to acquire DECO:
 225ece3e7dad4 crypto: caam - fix endless loop when DECO acquire fails
- add Fabio Estevam reviewed by tag in PATCH 3/3.
- removed CAAM ERA from dts since bootloader will add it - Horia.

V3:
- Added Cc: clk driver maintainers - Fabio Estevam
- Added Cc: i.MX arch maintainers - Fabio Estevam
- Removed bouncing email address for Herbert Xu

V2-resend:
- Patch 0005 lost in the ether - resending

V2:
- Endian detection is ok with TrustZone enabled Horia.
  Endian detection logic tested with TrustZone enabled. The register that
  this relies on though isn't affected by the lock-down in the first page.
  Assuming set of affected registers is actually just the 'deco' registers
  though there is no formal statement of that, that I am aware of.

- Moving of TrustZone work-around into u-boot
  This set actually doesn't need to deal with TrustZone at all now but, for
  the sake of consistency keeping thread title

  https://patchwork.ozlabs.org/patch/866460/
  https://patchwork.ozlabs.org/patch/866462/
  https://patchwork.ozlabs.org/patch/865890/

- Reworded endless loop fix to read a bit better

- Fixes to DTS additions - Rui

- Fixes to number of clocks declared - Rui

V1:
This patch-set enables CAAM on the i.MX7s and fixes a number of issues
identified with the CAAM driver and hardware when TrustZone mode is
enabled.

The first block of patches are simple bug-fixes, followed by a second block
of patches which are simple enabling patches for the i.MX7Solo - note we
aren't enabling for the i.MX7Dual since we don't have hardware to test that
out but it should be a 1:1 mapping for others to enable when appropriate.

Cheers,
Rui

Rui Miguel Silva (4):
  crypto: caam - Fix null dereference at error path
  crypto: caam - do not use mem and emi_slow clock for imx7x
  clk: imx7d: add CAAM clock
  ARM: dts: imx7s: add CAAM device node

 arch/arm/boot/dts/imx7s.dtsi| 30 +++
 drivers/clk/imx/clk-imx7d.c |  1 +
 drivers/crypto/caam/ctrl.c  | 42 +++--
 include/dt-bindings/clock/imx7d-clock.h |  3 ++-
 4 files changed, 57 insertions(+), 19 deletions(-)

-- 
2.16.2

[PATCH v4 0/4] Enable CAAM on i.MX7s fix TrustZone issues

[Rui Miguel Silva]

For v4 I am taking over this patch series as all the left over patches were
implemented by me.

V4:
- removed patch: [PATCH v3 2/5] crypto: caam: Fix endless loop when RNG is
already initialized from the series since Horia presented a better fix for the
 endless loop in case of fail to acquire DECO:
 225ece3e7dad4 crypto: caam - fix endless loop when DECO acquire fails
- add Fabio Estevam reviewed by tag in PATCH 3/3.
- removed CAAM ERA from dts since bootloader will add it - Horia.

V3:
- Added Cc: clk driver maintainers - Fabio Estevam
- Added Cc: i.MX arch maintainers - Fabio Estevam
- Removed bouncing email address for Herbert Xu

V2-resend:
- Patch 0005 lost in the ether - resending

V2:
- Endian detection is ok with TrustZone enabled Horia.
  Endian detection logic tested with TrustZone enabled. The register that
  this relies on though isn't affected by the lock-down in the first page.
  Assuming set of affected registers is actually just the 'deco' registers
  though there is no formal statement of that, that I am aware of.

- Moving of TrustZone work-around into u-boot
  This set actually doesn't need to deal with TrustZone at all now but, for
  the sake of consistency keeping thread title

  https://patchwork.ozlabs.org/patch/866460/
  https://patchwork.ozlabs.org/patch/866462/
  https://patchwork.ozlabs.org/patch/865890/

- Reworded endless loop fix to read a bit better

- Fixes to DTS additions - Rui

- Fixes to number of clocks declared - Rui

V1:
This patch-set enables CAAM on the i.MX7s and fixes a number of issues
identified with the CAAM driver and hardware when TrustZone mode is
enabled.

The first block of patches are simple bug-fixes, followed by a second block
of patches which are simple enabling patches for the i.MX7Solo - note we
aren't enabling for the i.MX7Dual since we don't have hardware to test that
out but it should be a 1:1 mapping for others to enable when appropriate.

Cheers,
Rui

Rui Miguel Silva (4):
  crypto: caam - Fix null dereference at error path
  crypto: caam - do not use mem and emi_slow clock for imx7x
  clk: imx7d: add CAAM clock
  ARM: dts: imx7s: add CAAM device node

 arch/arm/boot/dts/imx7s.dtsi| 30 +++
 drivers/clk/imx/clk-imx7d.c |  1 +
 drivers/crypto/caam/ctrl.c  | 42 +++--
 include/dt-bindings/clock/imx7d-clock.h |  3 ++-
 4 files changed, 57 insertions(+), 19 deletions(-)

-- 
2.16.2

[PATCH 2/2] media: ov2680: Add Omnivision OV2680 sensor driver

[Rui Miguel Silva]

This patch adds V4L2 sub-device driver for OV2680 image sensor.
The OV2680 is a 1/5" CMOS color sensor from Omnivision.
Supports output format: 10-bit Raw RGB.
The OV2680 has a single lane MIPI interface.

The driver exposes following V4L2 controls:
- auto/manual exposure,
- exposure,
- auto/manual gain,
- gain,
- horizontal/vertical flip,
- test pattern menu.
Supported resolution are only: QUXGA, 720P, UXGA.

Signed-off-by: Rui Miguel Silva <rui.si...@linaro.org>
---
 drivers/media/i2c/Kconfig  |   13 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov2680.c | 1189 
 3 files changed, 1203 insertions(+)
 create mode 100644 drivers/media/i2c/ov2680.c

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 9f18cd296841..089103d29171 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -586,6 +586,19 @@ config VIDEO_OV2659
  To compile this driver as a module, choose M here: the
  module will be called ov2659.
 
+config VIDEO_OV2680
+   tristate "OmniVision OV2680 sensor support"
+   depends on OF
+   depends on GPIOLIB && VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API
+   depends on MEDIA_CAMERA_SUPPORT
+   select V4L2_FWNODE
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the OmniVision
+ OV2680 camera sensor with a MIPI CSI-2 interface.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ov2680.
+
 config VIDEO_OV5640
tristate "OmniVision OV5640 sensor support"
depends on OF
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index c0f94cd8d56d..d0aba4d37b8d 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o
 obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
 obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
 obj-$(CONFIG_VIDEO_OV2640) += ov2640.o
+obj-$(CONFIG_VIDEO_OV2680) += ov2680.o
 obj-$(CONFIG_VIDEO_OV5640) += ov5640.o
 obj-$(CONFIG_VIDEO_OV5645) += ov5645.o
 obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c
new file mode 100644
index ..64c1c2b03f97
--- /dev/null
+++ b/drivers/media/i2c/ov2680.c
@@ -0,0 +1,1189 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Omnivision OV2680 CMOS Image Sensor driver
+ *
+ * Copyright (C) 2018 Linaro Ltd
+ *
+ * Based on OV5640 Sensor Driver
+ * Copyright (C) 2011-2013 Freescale Semiconductor, Inc. All Rights Reserved.
+ * Copyright (C) 2014-2017 Mentor Graphics Inc.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define OV2680_XVCLK_MIN   600
+#define OV2680_XVCLK_MAX   2400
+
+#define OV2680_CHIP_ID_HIGH0x26
+#define OV2680_CHIP_ID_LOW 0x80
+
+#define OV2680_REG_STREAM_CTRL 0x0100
+#define OV2680_REG_SOFT_RESET  0x0103
+
+#define OV2680_REG_CHIP_ID_HIGH0x300a
+#define OV2680_REG_CHIP_ID_LOW 0x300b
+
+#define OV2680_REG_R_MANUAL0x3503
+#define OV2680_REG_GAIN_PK 0x350a
+#define OV2680_REG_EXPOSURE_PK_HIGH0x3500
+#define OV2680_REG_EXPOSURE_PK_MED 0x3501
+#define OV2680_REG_EXPOSURE_PK_LOW 0x3502
+#define OV2680_REG_TIMING_HTS  0x380c
+#define OV2680_REG_TIMING_VTS  0x380e
+#define OV2680_REG_FORMAT1 0x3820
+#define OV2680_REG_FORMAT2 0x3821
+
+#define OV2680_REG_ISP_CTRL00  0x5080
+
+enum ov2680_frame_rate {
+   OV2680_30_FPS,
+   OV2680_FRAMERATES_MAX,
+};
+
+static const int ov2680_framerates[] = {
+   [OV2680_30_FPS] = 30,
+};
+
+enum ov2680_mode_id {
+   OV2680_MODE_QUXGA_800_600,
+   OV2680_MODE_720P_1280_720,
+   OV2680_MODE_UXGA_1600_1200,
+   OV2680_MODE_MAX,
+};
+
+struct reg_value {
+   u16 reg_addr;
+   u8 val;
+};
+
+struct ov2680_mode_info {
+   const char *name;
+   enum ov2680_mode_id id;
+   u32 width;
+   u32 height;
+   const struct reg_value *reg_data;
+   u32 reg_data_size;
+};
+
+struct ov2680_ctrls {
+   struct v4l2_ctrl_handler handler;
+   struct {
+   struct v4l2_ctrl *auto_exp;
+   struct v4l2_ctrl *exposure;
+   };
+   struct {
+   struct v4l2_ctrl *auto_gain;
+   struct v4l2_ctrl *gain;
+   };
+
+   struct v4l2_ctrl *hflip;
+   struct v4l2_ctrl *vflip;
+   struct v4l2_ctrl *test_pattern;
+};
+
+struct ov2680_dev {
+   struct i2c_client   *i2c_client;
+   struct v4l2_subdev  sd;
+
+   struct media_padpad;
+   struct v4l2_fwnode_endpoint ep;
+   struct clk  *xvclk;
+   u32

[PATCH 2/2] media: ov2680: Add Omnivision OV2680 sensor driver

[Rui Miguel Silva]

This patch adds V4L2 sub-device driver for OV2680 image sensor.
The OV2680 is a 1/5" CMOS color sensor from Omnivision.
Supports output format: 10-bit Raw RGB.
The OV2680 has a single lane MIPI interface.

The driver exposes following V4L2 controls:
- auto/manual exposure,
- exposure,
- auto/manual gain,
- gain,
- horizontal/vertical flip,
- test pattern menu.
Supported resolution are only: QUXGA, 720P, UXGA.

Signed-off-by: Rui Miguel Silva 
---
 drivers/media/i2c/Kconfig  |   13 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov2680.c | 1189 
 3 files changed, 1203 insertions(+)
 create mode 100644 drivers/media/i2c/ov2680.c

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 9f18cd296841..089103d29171 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -586,6 +586,19 @@ config VIDEO_OV2659
  To compile this driver as a module, choose M here: the
  module will be called ov2659.
 
+config VIDEO_OV2680
+   tristate "OmniVision OV2680 sensor support"
+   depends on OF
+   depends on GPIOLIB && VIDEO_V4L2 && I2C && VIDEO_V4L2_SUBDEV_API
+   depends on MEDIA_CAMERA_SUPPORT
+   select V4L2_FWNODE
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the OmniVision
+ OV2680 camera sensor with a MIPI CSI-2 interface.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ov2680.
+
 config VIDEO_OV5640
tristate "OmniVision OV5640 sensor support"
depends on OF
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index c0f94cd8d56d..d0aba4d37b8d 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_VIDEO_SONY_BTF_MPX) += sony-btf-mpx.o
 obj-$(CONFIG_VIDEO_UPD64031A) += upd64031a.o
 obj-$(CONFIG_VIDEO_UPD64083) += upd64083.o
 obj-$(CONFIG_VIDEO_OV2640) += ov2640.o
+obj-$(CONFIG_VIDEO_OV2680) += ov2680.o
 obj-$(CONFIG_VIDEO_OV5640) += ov5640.o
 obj-$(CONFIG_VIDEO_OV5645) += ov5645.o
 obj-$(CONFIG_VIDEO_OV5647) += ov5647.o
diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c
new file mode 100644
index ..64c1c2b03f97
--- /dev/null
+++ b/drivers/media/i2c/ov2680.c
@@ -0,0 +1,1189 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Omnivision OV2680 CMOS Image Sensor driver
+ *
+ * Copyright (C) 2018 Linaro Ltd
+ *
+ * Based on OV5640 Sensor Driver
+ * Copyright (C) 2011-2013 Freescale Semiconductor, Inc. All Rights Reserved.
+ * Copyright (C) 2014-2017 Mentor Graphics Inc.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define OV2680_XVCLK_MIN   600
+#define OV2680_XVCLK_MAX   2400
+
+#define OV2680_CHIP_ID_HIGH0x26
+#define OV2680_CHIP_ID_LOW 0x80
+
+#define OV2680_REG_STREAM_CTRL 0x0100
+#define OV2680_REG_SOFT_RESET  0x0103
+
+#define OV2680_REG_CHIP_ID_HIGH0x300a
+#define OV2680_REG_CHIP_ID_LOW 0x300b
+
+#define OV2680_REG_R_MANUAL0x3503
+#define OV2680_REG_GAIN_PK 0x350a
+#define OV2680_REG_EXPOSURE_PK_HIGH0x3500
+#define OV2680_REG_EXPOSURE_PK_MED 0x3501
+#define OV2680_REG_EXPOSURE_PK_LOW 0x3502
+#define OV2680_REG_TIMING_HTS  0x380c
+#define OV2680_REG_TIMING_VTS  0x380e
+#define OV2680_REG_FORMAT1 0x3820
+#define OV2680_REG_FORMAT2 0x3821
+
+#define OV2680_REG_ISP_CTRL00  0x5080
+
+enum ov2680_frame_rate {
+   OV2680_30_FPS,
+   OV2680_FRAMERATES_MAX,
+};
+
+static const int ov2680_framerates[] = {
+   [OV2680_30_FPS] = 30,
+};
+
+enum ov2680_mode_id {
+   OV2680_MODE_QUXGA_800_600,
+   OV2680_MODE_720P_1280_720,
+   OV2680_MODE_UXGA_1600_1200,
+   OV2680_MODE_MAX,
+};
+
+struct reg_value {
+   u16 reg_addr;
+   u8 val;
+};
+
+struct ov2680_mode_info {
+   const char *name;
+   enum ov2680_mode_id id;
+   u32 width;
+   u32 height;
+   const struct reg_value *reg_data;
+   u32 reg_data_size;
+};
+
+struct ov2680_ctrls {
+   struct v4l2_ctrl_handler handler;
+   struct {
+   struct v4l2_ctrl *auto_exp;
+   struct v4l2_ctrl *exposure;
+   };
+   struct {
+   struct v4l2_ctrl *auto_gain;
+   struct v4l2_ctrl *gain;
+   };
+
+   struct v4l2_ctrl *hflip;
+   struct v4l2_ctrl *vflip;
+   struct v4l2_ctrl *test_pattern;
+};
+
+struct ov2680_dev {
+   struct i2c_client   *i2c_client;
+   struct v4l2_subdev  sd;
+
+   struct media_padpad;
+   struct v4l2_fwnode_endpoint ep;
+   struct clk  *xvclk;
+   u32 xvclk_freq;
+
+

[PATCH 1/2] media: ov2680: dt: Add bindings for OV2680

[Rui Miguel Silva]

Add device tree binding documentation for the OV5640 camera sensor.

CC: devicet...@vger.kernel.org
Signed-off-by: Rui Miguel Silva <rui.si...@linaro.org>
---
 .../devicetree/bindings/media/i2c/ov2680.txt   | 34 ++
 1 file changed, 34 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/ov2680.txt 
b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
new file mode 100644
index ..f9dc63ce5044
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
@@ -0,0 +1,34 @@
+* Omnivision OV2680 MIPI CSI-2 sensor
+
+Required Properties:
+- compatible: should be "ovti,ov2680"
+- clocks: reference to the xvclk input clock.
+- clock-names: should be "xvclk".
+
+Optional Properties:
+- powerdown-gpios: reference to the GPIO connected to the powerdown pin,
+if any. This is an active high signal to the OV2680.
+
+The device node must contain one 'port' child node for its digital output
+video port, in accordance with the video interface bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Example:
+
+ {
+   ov2680: camera-sensor@36 {
+   compatible = "ovti,ov2680";
+   reg = <0x36>;
+   clocks = <>;
+   clock-names = "xvclk";
+   powerdown-gpios = < 3 GPIO_ACTIVE_HIGH>;
+
+   port {
+   ov2680_mipi_ep: endpoint {
+   remote-endpoint = <_sensor_ep>;
+   clock-lanes = <0>;
+   data-lanes = <1>;
+   };
+   };
+   };
+};
-- 
2.16.2

[PATCH 1/2] media: ov2680: dt: Add bindings for OV2680

[Rui Miguel Silva]

Add device tree binding documentation for the OV5640 camera sensor.

CC: devicet...@vger.kernel.org
Signed-off-by: Rui Miguel Silva 
---
 .../devicetree/bindings/media/i2c/ov2680.txt   | 34 ++
 1 file changed, 34 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt

diff --git a/Documentation/devicetree/bindings/media/i2c/ov2680.txt 
b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
new file mode 100644
index ..f9dc63ce5044
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
@@ -0,0 +1,34 @@
+* Omnivision OV2680 MIPI CSI-2 sensor
+
+Required Properties:
+- compatible: should be "ovti,ov2680"
+- clocks: reference to the xvclk input clock.
+- clock-names: should be "xvclk".
+
+Optional Properties:
+- powerdown-gpios: reference to the GPIO connected to the powerdown pin,
+if any. This is an active high signal to the OV2680.
+
+The device node must contain one 'port' child node for its digital output
+video port, in accordance with the video interface bindings defined in
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Example:
+
+ {
+   ov2680: camera-sensor@36 {
+   compatible = "ovti,ov2680";
+   reg = <0x36>;
+   clocks = <>;
+   clock-names = "xvclk";
+   powerdown-gpios = < 3 GPIO_ACTIVE_HIGH>;
+
+   port {
+   ov2680_mipi_ep: endpoint {
+   remote-endpoint = <_sensor_ep>;
+   clock-lanes = <0>;
+   data-lanes = <1>;
+   };
+   };
+   };
+};
-- 
2.16.2

[PATCH 0/2] media: Introduce Omnivision OV2680 driver

[Rui Miguel Silva]

Add driver and bindings for the OV2680 2 megapixel CMOS 1/5" sensor, which has
a single MIPI lane interface and output format of 10-bit Raw RGB.

Features supported are described in PATCH 2/2.

Cheers,
Rui

Rui Miguel Silva (2):
  media: ov2680: dt: Add bindings for OV2680
  media: ov2680: Add Omnivision OV2680 sensor driver

 .../devicetree/bindings/media/i2c/ov2680.txt   |   34 +
 drivers/media/i2c/Kconfig  |   13 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov2680.c | 1189 
 4 files changed, 1237 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt
 create mode 100644 drivers/media/i2c/ov2680.c

-- 
2.16.2

[PATCH 0/2] media: Introduce Omnivision OV2680 driver

[Rui Miguel Silva]

Add driver and bindings for the OV2680 2 megapixel CMOS 1/5" sensor, which has
a single MIPI lane interface and output format of 10-bit Raw RGB.

Features supported are described in PATCH 2/2.

Cheers,
Rui

Rui Miguel Silva (2):
  media: ov2680: dt: Add bindings for OV2680
  media: ov2680: Add Omnivision OV2680 sensor driver

 .../devicetree/bindings/media/i2c/ov2680.txt   |   34 +
 drivers/media/i2c/Kconfig  |   13 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov2680.c | 1189 
 4 files changed, 1237 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt
 create mode 100644 drivers/media/i2c/ov2680.c

-- 
2.16.2

Re: [RESEND PATCH 3/6] ARM: dts: imx7s: add CAAM device node

[Rui Miguel Silva]

Hi,
Thanks for the report.
On Sat 27 Jan 2018 at 15:49, kbuild test robot wrote:
> Hi Rui,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on crypto/master]
> [also build test ERROR on v4.15-rc9 next-20180126]
> [cannot apply to cryptodev/master]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
>
> url:
> https://github.com/0day-ci/linux/commits/Bryan-O-Donoghue/Enable-CAAM-on-i-MX7s-fix-TrustZone-issues/20180127-185422
> base:   
> https://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6.git master
> config: arm-u8500_defconfig (attached as .config)
> compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
> reproduce:
> wget 
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
> ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=arm 
>
> Note: the 
> linux-review/Bryan-O-Donoghue/Enable-CAAM-on-i-MX7s-fix-TrustZone-issues/20180127-185422
>  HEAD f907a172373d2c61dd7bf25a88621abc6e410f15 builds fine.
>   It only hurts bisectibility.

Yeah, the order of the patches in the series were wrong and break
bisectibility.

V2 of this series already sent, besides other fixes, also fix this.
Once again many thanks.

---
Cheers,
Rui

Re: [RESEND PATCH 3/6] ARM: dts: imx7s: add CAAM device node

[Rui Miguel Silva]

Hi,
Thanks for the report.
On Sat 27 Jan 2018 at 15:49, kbuild test robot wrote:
> Hi Rui,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on crypto/master]
> [also build test ERROR on v4.15-rc9 next-20180126]
> [cannot apply to cryptodev/master]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
>
> url:
> https://github.com/0day-ci/linux/commits/Bryan-O-Donoghue/Enable-CAAM-on-i-MX7s-fix-TrustZone-issues/20180127-185422
> base:   
> https://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6.git master
> config: arm-u8500_defconfig (attached as .config)
> compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
> reproduce:
> wget 
> https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
> ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> make.cross ARCH=arm 
>
> Note: the 
> linux-review/Bryan-O-Donoghue/Enable-CAAM-on-i-MX7s-fix-TrustZone-issues/20180127-185422
>  HEAD f907a172373d2c61dd7bf25a88621abc6e410f15 builds fine.
>   It only hurts bisectibility.

Yeah, the order of the patches in the series were wrong and break
bisectibility.

V2 of this series already sent, besides other fixes, also fix this.
Once again many thanks.

---
Cheers,
Rui

Re: [PATCH] drivers/staging/greybus: fix max dup length for kstrndup

[Rui Miguel Silva]

Hi Ma,
Thanks for your patch.

Please make sure you use scripts/get_maintainer.pl so that your patches
goes to the right lists and people. I CC's them now. ;)


On Tue 12 Dec 2017 at 09:25, Ma Shimiao wrote:
> If source string longer than max, kstrndup will alloc max+1 space.
> So, we should make sure the result will not over limit.

I think we are good here. kstrndup alloc memory for us the max+1, and
in fact we want to have the 32 chars plus the \0 set by kstrndup.

So, I think the code as is now is ok.

---
Cheers,
Rui

>
> Signed-off-by: Ma Shimiao 
> ---
>  drivers/staging/greybus/light.c|  9 ++---
>  drivers/staging/greybus/power_supply.c | 10 ++
>  2 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c
> index 010ae1e9c7fb..c7ac2ead5a07 100644
> --- a/drivers/staging/greybus/light.c
> +++ b/drivers/staging/greybus/light.c
> @@ -965,10 +965,12 @@ static int gb_lights_channel_config(struct gb_light 
> *light,
>   channel->mode = le32_to_cpu(conf.mode);
>   channel->flags = le32_to_cpu(conf.flags);
>   channel->color = le32_to_cpu(conf.color);
> - channel->color_name = kstrndup(conf.color_name, NAMES_MAX, GFP_KERNEL);
> + channel->color_name = kstrndup(conf.color_name,
> +NAMES_MAX - 1, GFP_KERNEL);
>   if (!channel->color_name)
>   return -ENOMEM;
> - channel->mode_name = kstrndup(conf.mode_name, NAMES_MAX, GFP_KERNEL);
> + channel->mode_name = kstrndup(conf.mode_name,
> +   NAMES_MAX - 1, GFP_KERNEL);
>   if (!channel->mode_name)
>   return -ENOMEM;
>  
> @@ -1027,7 +1029,8 @@ static int gb_lights_light_config(struct gb_lights 
> *glights, u8 id)
>   return -EINVAL;
>  
>   light->channels_count = conf.channel_count;
> - light->name = kstrndup(conf.name, NAMES_MAX, GFP_KERNEL);
> + light->name = kstrndup(conf.name,
> +NAMES_MAX - 1, GFP_KERNEL);
>  
>   light->channels = kcalloc(light->channels_count,
> sizeof(struct gb_channel), GFP_KERNEL);
> diff --git a/drivers/staging/greybus/power_supply.c 
> b/drivers/staging/greybus/power_supply.c
> index 0529e5628c24..7bc76633866b 100644
> --- a/drivers/staging/greybus/power_supply.c
> +++ b/drivers/staging/greybus/power_supply.c
> @@ -487,14 +487,16 @@ static int gb_power_supply_description_get(struct 
> gb_power_supply *gbpsy)
>   if (ret < 0)
>   return ret;
>  
> - gbpsy->manufacturer = kstrndup(resp.manufacturer, PROP_MAX, GFP_KERNEL);
> + gbpsy->manufacturer = kstrndup(resp.manufacturer,
> +PROP_MAX - 1, GFP_KERNEL);
>   if (!gbpsy->manufacturer)
>   return -ENOMEM;
> - gbpsy->model_name = kstrndup(resp.model, PROP_MAX, GFP_KERNEL);
> + gbpsy->model_name = kstrndup(resp.model,
> +  PROP_MAX - 1, GFP_KERNEL);
>   if (!gbpsy->model_name)
>   return -ENOMEM;
> - gbpsy->serial_number = kstrndup(resp.serial_number, PROP_MAX,
> -GFP_KERNEL);
> + gbpsy->serial_number = kstrndup(resp.serial_number,
> + PROP_MAX - 1, GFP_KERNEL);
>   if (!gbpsy->serial_number)
>   return -ENOMEM;

Re: [PATCH] drivers/staging/greybus: fix max dup length for kstrndup

[Rui Miguel Silva]

Hi Ma,
Thanks for your patch.

Please make sure you use scripts/get_maintainer.pl so that your patches
goes to the right lists and people. I CC's them now. ;)


On Tue 12 Dec 2017 at 09:25, Ma Shimiao wrote:
> If source string longer than max, kstrndup will alloc max+1 space.
> So, we should make sure the result will not over limit.

I think we are good here. kstrndup alloc memory for us the max+1, and
in fact we want to have the 32 chars plus the \0 set by kstrndup.

So, I think the code as is now is ok.

---
Cheers,
Rui

>
> Signed-off-by: Ma Shimiao 
> ---
>  drivers/staging/greybus/light.c|  9 ++---
>  drivers/staging/greybus/power_supply.c | 10 ++
>  2 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c
> index 010ae1e9c7fb..c7ac2ead5a07 100644
> --- a/drivers/staging/greybus/light.c
> +++ b/drivers/staging/greybus/light.c
> @@ -965,10 +965,12 @@ static int gb_lights_channel_config(struct gb_light 
> *light,
>   channel->mode = le32_to_cpu(conf.mode);
>   channel->flags = le32_to_cpu(conf.flags);
>   channel->color = le32_to_cpu(conf.color);
> - channel->color_name = kstrndup(conf.color_name, NAMES_MAX, GFP_KERNEL);
> + channel->color_name = kstrndup(conf.color_name,
> +NAMES_MAX - 1, GFP_KERNEL);
>   if (!channel->color_name)
>   return -ENOMEM;
> - channel->mode_name = kstrndup(conf.mode_name, NAMES_MAX, GFP_KERNEL);
> + channel->mode_name = kstrndup(conf.mode_name,
> +   NAMES_MAX - 1, GFP_KERNEL);
>   if (!channel->mode_name)
>   return -ENOMEM;
>  
> @@ -1027,7 +1029,8 @@ static int gb_lights_light_config(struct gb_lights 
> *glights, u8 id)
>   return -EINVAL;
>  
>   light->channels_count = conf.channel_count;
> - light->name = kstrndup(conf.name, NAMES_MAX, GFP_KERNEL);
> + light->name = kstrndup(conf.name,
> +NAMES_MAX - 1, GFP_KERNEL);
>  
>   light->channels = kcalloc(light->channels_count,
> sizeof(struct gb_channel), GFP_KERNEL);
> diff --git a/drivers/staging/greybus/power_supply.c 
> b/drivers/staging/greybus/power_supply.c
> index 0529e5628c24..7bc76633866b 100644
> --- a/drivers/staging/greybus/power_supply.c
> +++ b/drivers/staging/greybus/power_supply.c
> @@ -487,14 +487,16 @@ static int gb_power_supply_description_get(struct 
> gb_power_supply *gbpsy)
>   if (ret < 0)
>   return ret;
>  
> - gbpsy->manufacturer = kstrndup(resp.manufacturer, PROP_MAX, GFP_KERNEL);
> + gbpsy->manufacturer = kstrndup(resp.manufacturer,
> +PROP_MAX - 1, GFP_KERNEL);
>   if (!gbpsy->manufacturer)
>   return -ENOMEM;
> - gbpsy->model_name = kstrndup(resp.model, PROP_MAX, GFP_KERNEL);
> + gbpsy->model_name = kstrndup(resp.model,
> +  PROP_MAX - 1, GFP_KERNEL);
>   if (!gbpsy->model_name)
>   return -ENOMEM;
> - gbpsy->serial_number = kstrndup(resp.serial_number, PROP_MAX,
> -GFP_KERNEL);
> + gbpsy->serial_number = kstrndup(resp.serial_number,
> + PROP_MAX - 1, GFP_KERNEL);
>   if (!gbpsy->serial_number)
>   return -ENOMEM;

Re: [PATCH] staging: greybus: spilib: fix use-after-free after deregistration

[Rui Miguel Silva]

Hi Johan,
Thanks for the patch.

On Sun 29 Oct 2017 at 12:01, Johan Hovold <jo...@kernel.org> wrote:
> Remove erroneous spi_master_put() after controller deregistration which
> would access the already freed spi controller.
>
> Note that spi_unregister_master() drops our only controller reference.
>
> Fixes: ba3e67001b42 ("greybus: SPI: convert to a gpbridge driver")
> Cc: stable <sta...@vger.kernel.org> # 4.9
> Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
> Signed-off-by: Johan Hovold <jo...@kernel.org>

Reviewed-by: Rui Miguel Silva <rmf...@gmail.com>

---
Cheers,
Rui

> ---
>  drivers/staging/greybus/spilib.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/greybus/spilib.c 
> b/drivers/staging/greybus/spilib.c
> index e97b19148497..1e7321a1404c 100644
> --- a/drivers/staging/greybus/spilib.c
> +++ b/drivers/staging/greybus/spilib.c
> @@ -544,11 +544,14 @@ int gb_spilib_master_init(struct gb_connection 
> *connection, struct device *dev,
>  
>   return 0;
>  
> -exit_spi_unregister:
> - spi_unregister_master(master);
>  exit_spi_put:
>   spi_master_put(master);
>  
> + return ret;
> +
> +exit_spi_unregister:
> + spi_unregister_master(master);
> +
>   return ret;
>  }
>  EXPORT_SYMBOL_GPL(gb_spilib_master_init);
> @@ -558,7 +561,6 @@ void gb_spilib_master_exit(struct gb_connection 
> *connection)
>   struct spi_master *master = gb_connection_get_data(connection);
>  
>   spi_unregister_master(master);
> - spi_master_put(master);
>  }
>  EXPORT_SYMBOL_GPL(gb_spilib_master_exit);

Re: [PATCH] staging: greybus: spilib: fix use-after-free after deregistration

[Rui Miguel Silva]

Hi Johan,
Thanks for the patch.

On Sun 29 Oct 2017 at 12:01, Johan Hovold  wrote:
> Remove erroneous spi_master_put() after controller deregistration which
> would access the already freed spi controller.
>
> Note that spi_unregister_master() drops our only controller reference.
>
> Fixes: ba3e67001b42 ("greybus: SPI: convert to a gpbridge driver")
> Cc: stable  # 4.9
> Cc: Greg Kroah-Hartman 
> Signed-off-by: Johan Hovold 

Reviewed-by: Rui Miguel Silva 

---
Cheers,
Rui

> ---
>  drivers/staging/greybus/spilib.c | 8 +---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/greybus/spilib.c 
> b/drivers/staging/greybus/spilib.c
> index e97b19148497..1e7321a1404c 100644
> --- a/drivers/staging/greybus/spilib.c
> +++ b/drivers/staging/greybus/spilib.c
> @@ -544,11 +544,14 @@ int gb_spilib_master_init(struct gb_connection 
> *connection, struct device *dev,
>  
>   return 0;
>  
> -exit_spi_unregister:
> - spi_unregister_master(master);
>  exit_spi_put:
>   spi_master_put(master);
>  
> + return ret;
> +
> +exit_spi_unregister:
> + spi_unregister_master(master);
> +
>   return ret;
>  }
>  EXPORT_SYMBOL_GPL(gb_spilib_master_init);
> @@ -558,7 +561,6 @@ void gb_spilib_master_exit(struct gb_connection 
> *connection)
>   struct spi_master *master = gb_connection_get_data(connection);
>  
>   spi_unregister_master(master);
> - spi_master_put(master);
>  }
>  EXPORT_SYMBOL_GPL(gb_spilib_master_exit);

Re: [PATCH] staging: greybus: light: remove unnecessary error check

[Rui Miguel Silva]

Hi,
On Sat, Sep 23, 2017 at 01:39:15PM +0530, Arvind Yadav wrote:
> It is not necessary to check return value of gb_lights_channel_flash_config.
> gb_lights_channel_config returns both successful and error value.
> 
> Signed-off-by: Arvind Yadav <arvind.yadav...@gmail.com>

Thanks for the patch.
Reviewed-by: Rui Miguel Silva <rmf...@gmail.com>

---
Cheers,
Rui

> ---
>  drivers/staging/greybus/light.c | 6 +-
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c
> index 0f538b8..d7da475 100644
> --- a/drivers/staging/greybus/light.c
> +++ b/drivers/staging/greybus/light.c
> @@ -1000,11 +1000,7 @@ static int gb_lights_channel_config(struct gb_light 
> *light,
>  
>   light->has_flash = true;
>  
> - ret = gb_lights_channel_flash_config(channel);
> - if (ret < 0)
> - return ret;
> -
> - return ret;
> + return gb_lights_channel_flash_config(channel);
>  }
>  
>  static int gb_lights_light_config(struct gb_lights *glights, u8 id)
> -- 
> 2.7.4
> 

Re: [PATCH] staging: greybus: light: remove unnecessary error check

[Rui Miguel Silva]

Hi,
On Sat, Sep 23, 2017 at 01:39:15PM +0530, Arvind Yadav wrote:
> It is not necessary to check return value of gb_lights_channel_flash_config.
> gb_lights_channel_config returns both successful and error value.
> 
> Signed-off-by: Arvind Yadav 

Thanks for the patch.
Reviewed-by: Rui Miguel Silva 

---
Cheers,
Rui

> ---
>  drivers/staging/greybus/light.c | 6 +-
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c
> index 0f538b8..d7da475 100644
> --- a/drivers/staging/greybus/light.c
> +++ b/drivers/staging/greybus/light.c
> @@ -1000,11 +1000,7 @@ static int gb_lights_channel_config(struct gb_light 
> *light,
>  
>   light->has_flash = true;
>  
> - ret = gb_lights_channel_flash_config(channel);
> - if (ret < 0)
> - return ret;
> -
> - return ret;
> + return gb_lights_channel_flash_config(channel);
>  }
>  
>  static int gb_lights_light_config(struct gb_lights *glights, u8 id)
> -- 
> 2.7.4
> 

Re: [PATCH v3] staging: greybus: light: Release memory obtained by kasprintf

[Rui Miguel Silva]

Hi,
On Sat, Sep 23, 2017 at 01:25:30PM +0530, Arvind Yadav wrote:
> Free memory region, if gb_lights_channel_config is not successful.
> 
> Signed-off-by: Arvind Yadav <arvind.yadav...@gmail.com>

Thanks for the patch, looks good to me.

Reviewed-by: Rui Miguel Silva <rmf...@gmail.com>

---
Cheers,
Rui

> ---
> changes in v2:
> - Subject line changed.
> - add kfree in __gb_lights_led_unregister().
> - No need to check return value of 
> gb_lights_channel_flash_config().
> 
> changes ib v3:
> - separate patch for "No need to check return value of
>   gb_lights_channel_flash_config()".
> 
>  drivers/staging/greybus/light.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c
> index 3f4148c..0f538b8 100644
> --- a/drivers/staging/greybus/light.c
> +++ b/drivers/staging/greybus/light.c
> @@ -925,6 +925,8 @@ static void __gb_lights_led_unregister(struct gb_channel 
> *channel)
>   return;
>  
>   led_classdev_unregister(cdev);
> + kfree(cdev->name);
> + cdev->name = NULL;
>   channel->led = NULL;
>  }
>  
> -- 
> 2.7.4
> 

Re: [PATCH v3] staging: greybus: light: Release memory obtained by kasprintf

[Rui Miguel Silva]

Hi,
On Sat, Sep 23, 2017 at 01:25:30PM +0530, Arvind Yadav wrote:
> Free memory region, if gb_lights_channel_config is not successful.
> 
> Signed-off-by: Arvind Yadav 

Thanks for the patch, looks good to me.

Reviewed-by: Rui Miguel Silva 

---
Cheers,
Rui

> ---
> changes in v2:
> - Subject line changed.
> - add kfree in __gb_lights_led_unregister().
> - No need to check return value of 
> gb_lights_channel_flash_config().
> 
> changes ib v3:
> - separate patch for "No need to check return value of
>   gb_lights_channel_flash_config()".
> 
>  drivers/staging/greybus/light.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c
> index 3f4148c..0f538b8 100644
> --- a/drivers/staging/greybus/light.c
> +++ b/drivers/staging/greybus/light.c
> @@ -925,6 +925,8 @@ static void __gb_lights_led_unregister(struct gb_channel 
> *channel)
>   return;
>  
>   led_classdev_unregister(cdev);
> + kfree(cdev->name);
> + cdev->name = NULL;
>   channel->led = NULL;
>  }
>  
> -- 
> 2.7.4
> 

Re: [PATCH] media: staging: greybus: Release memory obtained by kasprintf

[Rui Miguel Silva]

Hi,
On Thu, Sep 21, 2017 at 03:59:18PM +0300, Dan Carpenter wrote:
> On Thu, Sep 21, 2017 at 05:05:27PM +0530, Arvind Yadav wrote:
> > Free memory region, if gb_lights_channel_config is not successful.

Arvind, thanks for patch and good catch.
But please look at the subject of other patches applied to this
file and try to stick with the labels, staging: greybus: light:

> > 
> 
> The question I have is do we free this on module unload?  I don't see
> that we do.  I feel like we should do a free after calling
> __gb_lights_led_unregister().  But that's awkward because we call
> __gb_lights_led_unregister() when this function fails so it would end
> up being a double free.

Yes Dan, You are correct, this should be free in
__gb_lights_led_unregister(), and not here.

> 
> > Signed-off-by: Arvind Yadav 
> > ---
> >  drivers/staging/greybus/light.c | 9 ++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/staging/greybus/light.c 
> > b/drivers/staging/greybus/light.c
> > index 3f4148c..b00d47c 100644
> > --- a/drivers/staging/greybus/light.c
> > +++ b/drivers/staging/greybus/light.c
> > @@ -984,7 +984,7 @@ static int gb_lights_channel_config(struct gb_light 
> > *light,
> >  
> > ret = channel_attr_groups_set(channel, cdev);
> > if (ret < 0)
> > -   return ret;
> > +   goto err;
> >  
> > gb_lights_led_operations_set(channel, cdev);
> >  
> > @@ -994,15 +994,18 @@ static int gb_lights_channel_config(struct gb_light 
> > *light,
> >  * configurations.
> >  */
> > if (!is_channel_flash(channel))
> > -   return ret;
> > +   goto err;
> 
> "ret" is zero here.  This is actually a success return.  It would be
> cleaner to just write "return 0;".  Anyway, this patch introduces a use
> after free so that doesn't work.
> 
> Also it's better to choose a label name which says what the label does
> so in this case it would be "goto err_free_name" or "goto err_cdev_name"
> or whatever, but something to indicate that it's to do with freeing
> the cdev->name.  Just "err" is too ambiguous.
> 
> >  
> > light->has_flash = true;
> >  
> > ret = gb_lights_channel_flash_config(channel);
> > if (ret < 0)
> > -   return ret;
> > +   goto err;
> >  
> > return ret;
> ^^
> Here as well, change this from "return ret;" to "return 0;".

It should be return 0; from the start, you are right, but that
would be a complete different change than the actual fix that now
goes far away from this code.

Thank both,
---
Cheers,
Rui

Re: [PATCH] media: staging: greybus: Release memory obtained by kasprintf

[Rui Miguel Silva]

Hi,
On Thu, Sep 21, 2017 at 03:59:18PM +0300, Dan Carpenter wrote:
> On Thu, Sep 21, 2017 at 05:05:27PM +0530, Arvind Yadav wrote:
> > Free memory region, if gb_lights_channel_config is not successful.

Arvind, thanks for patch and good catch.
But please look at the subject of other patches applied to this
file and try to stick with the labels, staging: greybus: light:

> > 
> 
> The question I have is do we free this on module unload?  I don't see
> that we do.  I feel like we should do a free after calling
> __gb_lights_led_unregister().  But that's awkward because we call
> __gb_lights_led_unregister() when this function fails so it would end
> up being a double free.

Yes Dan, You are correct, this should be free in
__gb_lights_led_unregister(), and not here.

> 
> > Signed-off-by: Arvind Yadav 
> > ---
> >  drivers/staging/greybus/light.c | 9 ++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/staging/greybus/light.c 
> > b/drivers/staging/greybus/light.c
> > index 3f4148c..b00d47c 100644
> > --- a/drivers/staging/greybus/light.c
> > +++ b/drivers/staging/greybus/light.c
> > @@ -984,7 +984,7 @@ static int gb_lights_channel_config(struct gb_light 
> > *light,
> >  
> > ret = channel_attr_groups_set(channel, cdev);
> > if (ret < 0)
> > -   return ret;
> > +   goto err;
> >  
> > gb_lights_led_operations_set(channel, cdev);
> >  
> > @@ -994,15 +994,18 @@ static int gb_lights_channel_config(struct gb_light 
> > *light,
> >  * configurations.
> >  */
> > if (!is_channel_flash(channel))
> > -   return ret;
> > +   goto err;
> 
> "ret" is zero here.  This is actually a success return.  It would be
> cleaner to just write "return 0;".  Anyway, this patch introduces a use
> after free so that doesn't work.
> 
> Also it's better to choose a label name which says what the label does
> so in this case it would be "goto err_free_name" or "goto err_cdev_name"
> or whatever, but something to indicate that it's to do with freeing
> the cdev->name.  Just "err" is too ambiguous.
> 
> >  
> > light->has_flash = true;
> >  
> > ret = gb_lights_channel_flash_config(channel);
> > if (ret < 0)
> > -   return ret;
> > +   goto err;
> >  
> > return ret;
> ^^
> Here as well, change this from "return ret;" to "return 0;".

It should be return 0; from the start, you are right, but that
would be a complete different change than the actual fix that now
goes far away from this code.

Thank both,
---
Cheers,
Rui

Re: [PATCH] staging: greybus: power_supply: replace kzalloc by kcalloc

[Rui Miguel Silva]

Hi JB,
Great! thanks for the patch,
On Thu, May 11, 2017 at 10:58:56PM +0200, JB Van Puyvelde wrote:

According to checkpatch.pl, kcalloc should be preferred to kzalloc with
multiply.

Signed-off-by: JB Van Puyvelde <jbvanpuyve...@gmail.com>

Reviewed-by: Rui Miguel Silva <rmf...@gmail.com>

Cheers,
  Rui


---
drivers/staging/greybus/power_supply.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/greybus/power_supply.c 
b/drivers/staging/greybus/power_supply.c
index e85c988b7034..20cac20518d7 100644
--- a/drivers/staging/greybus/power_supply.c
+++ b/drivers/staging/greybus/power_supply.c
@@ -944,7 +944,7 @@ static int gb_power_supplies_setup(struct gb_power_supplies 
*supplies)
if (ret < 0)
goto out;

-   supplies->supply = kzalloc(supplies->supplies_count *
+   supplies->supply = kcalloc(supplies->supplies_count,
 sizeof(struct gb_power_supply),
 GFP_KERNEL);

--
2.11.0

Re: [PATCH] staging: greybus: power_supply: replace kzalloc by kcalloc

[Rui Miguel Silva]

Hi JB,
Great! thanks for the patch,
On Thu, May 11, 2017 at 10:58:56PM +0200, JB Van Puyvelde wrote:

According to checkpatch.pl, kcalloc should be preferred to kzalloc with
multiply.

Signed-off-by: JB Van Puyvelde 

Reviewed-by: Rui Miguel Silva 

Cheers,
  Rui


---
drivers/staging/greybus/power_supply.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/greybus/power_supply.c 
b/drivers/staging/greybus/power_supply.c
index e85c988b7034..20cac20518d7 100644
--- a/drivers/staging/greybus/power_supply.c
+++ b/drivers/staging/greybus/power_supply.c
@@ -944,7 +944,7 @@ static int gb_power_supplies_setup(struct gb_power_supplies 
*supplies)
if (ret < 0)
goto out;

-   supplies->supply = kzalloc(supplies->supplies_count *
+   supplies->supply = kcalloc(supplies->supplies_count,
 sizeof(struct gb_power_supply),
 GFP_KERNEL);

--
2.11.0