Re: [PATCH v3] media: davinci vpbe: array underflow in vpbe_enum_outputs()

[Lad, Prabhakar]

On Mon, May 28, 2018 at 7:26 AM, Dan Carpenter  wrote:
> In vpbe_enum_outputs() we check if (temp_index >= cfg->num_outputs) but
> the problem is that temp_index can be negative.  I've changed the types
> to unsigned to fix this issue.
>
> Fixes: 66715cdc3224 ("[media] davinci vpbe: VPBE display driver")
> Signed-off-by: Dan Carpenter 
> ---
> v2: fix it a different way
> v3: change everything to unsigned because that's the right thing to do
>     and looks nicer.
>

Acked-by: Lad, Prabhakar 

Regards,
--Prabhakar Lad

> diff --git a/include/media/davinci/vpbe.h b/include/media/davinci/vpbe.h
> index 79a566d7defd..180a05e91497 100644
> --- a/include/media/davinci/vpbe.h
> +++ b/include/media/davinci/vpbe.h
> @@ -92,7 +92,7 @@ struct vpbe_config {
> struct encoder_config_info *ext_encoders;
> /* amplifier information goes here */
> struct amp_config_info *amp;
> -   int num_outputs;
> +   unsigned int num_outputs;
> /* Order is venc outputs followed by LCD and then external encoders */
> struct vpbe_output *outputs;
>  };
> diff --git a/drivers/media/platform/davinci/vpbe.c 
> b/drivers/media/platform/davinci/vpbe.c
> index 18c035ef84cf..c6fee53bff4d 100644
> --- a/drivers/media/platform/davinci/vpbe.c
> +++ b/drivers/media/platform/davinci/vpbe.c
> @@ -126,7 +126,7 @@ static int vpbe_enum_outputs(struct vpbe_device *vpbe_dev,
>  struct v4l2_output *output)
>  {
> struct vpbe_config *cfg = vpbe_dev->cfg;
> -   int temp_index = output->index;
> +   unsigned int temp_index = output->index;
>
> if (temp_index >= cfg->num_outputs)
> return -EINVAL;

Re: [PATCH v2 3/8] media: v4l2-async: simplify v4l2_async_subdev structure

[Lad, Prabhakar]

Hi Mauro,

Thanks for the patch.

On Tue, Dec 19, 2017 at 11:18 AM, Mauro Carvalho Chehab
<mche...@s-opensource.com> wrote:
> The V4L2_ASYNC_MATCH_FWNODE match criteria requires just one
> struct to be filled (struct fwnode_handle). The V4L2_ASYNC_MATCH_DEVNAME
> match criteria requires just a device name.
>
> So, it doesn't make sense to enclose those into structs,
> as the criteria can go directly into the union.
>
> That makes easier to document it, as we don't need to document
> weird senseless structs.
>
> At drivers, this makes even clearer about the match criteria.
>
> Acked-by: Sylwester Nawrocki <s.nawro...@samsung.com>
> Acked-by: Benoit Parrot <bpar...@ti.com>
> Acked-by: Alexandre Belloni <alexandre.bell...@free-electrons.com>
> Acked-by: Sakari Ailus <sakari.ai...@linux.intel.com>
> Acked-by: Philipp Zabel <p.za...@pengutronix.de>
> Signed-off-by: Mauro Carvalho Chehab <mche...@s-opensource.com>
> ---
>  drivers/media/platform/am437x/am437x-vpfe.c|  6 +++---

For above:

Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Cheers,
--Prabhakar Lad

Re: [PATCH 4/7] media: davinci: fix kernel-doc warnings

[Lad, Prabhakar]

HI Mauro,

Thanks for the patch.

On Wed, Nov 29, 2017 at 12:08 PM, Mauro Carvalho Chehab
<mche...@s-opensource.com> wrote:
> There are several of kernel-doc warnings:
>
> drivers/media/platform/davinci/vpif_display.c:114: warning: No 
> description found for parameter 'sizes'
> drivers/media/platform/davinci/vpif_display.c:165: warning: No 
> description found for parameter 'vq'
> drivers/media/platform/davinci/vpif_display.c:165: warning: Excess 
> function parameter 'vb' description in 'vpif_start_streaming'
> drivers/media/platform/davinci/vpif_display.c:780: warning: No 
> description found for parameter 'vpif_cfg'
> drivers/media/platform/davinci/vpif_display.c:780: warning: No 
> description found for parameter 'chan_cfg'
> drivers/media/platform/davinci/vpif_display.c:780: warning: No 
> description found for parameter 'index'
> drivers/media/platform/davinci/vpif_display.c:813: warning: No 
> description found for parameter 'vpif_cfg'
> drivers/media/platform/davinci/vpif_display.c:813: warning: No 
> description found for parameter 'ch'
> drivers/media/platform/davinci/vpif_display.c:813: warning: No 
> description found for parameter 'index'
> drivers/media/platform/davinci/vpif_capture.c:121: warning: No 
> description found for parameter 'sizes'
> drivers/media/platform/davinci/vpif_capture.c:174: warning: No 
> description found for parameter 'vq'
> drivers/media/platform/davinci/vpif_capture.c:174: warning: Excess 
> function parameter 'vb' description in 'vpif_start_streaming'
> drivers/media/platform/davinci/vpif_capture.c:636: warning: No 
> description found for parameter 'iface'
> drivers/media/platform/davinci/vpif_capture.c:647: warning: No 
> description found for parameter 'ch'
> drivers/media/platform/davinci/vpif_capture.c:647: warning: No 
> description found for parameter 'muxmode'
> drivers/media/platform/davinci/vpif_capture.c:676: warning: No 
> description found for parameter 'vpif_cfg'
> drivers/media/platform/davinci/vpif_capture.c:676: warning: No 
> description found for parameter 'chan_cfg'
> drivers/media/platform/davinci/vpif_capture.c:676: warning: No 
> description found for parameter 'input_index'
> drivers/media/platform/davinci/vpif_capture.c:712: warning: No 
> description found for parameter 'vpif_cfg'
> drivers/media/platform/davinci/vpif_capture.c:712: warning: No 
> description found for parameter 'ch'
> drivers/media/platform/davinci/vpif_capture.c:712: warning: No 
> description found for parameter 'index'
> drivers/media/platform/davinci/vpif_capture.c:798: warning: No 
> description found for parameter 'std'
> drivers/media/platform/davinci/vpif_capture.c:798: warning: Excess 
> function parameter 'std_id' description in 'vpif_g_std'
> drivers/media/platform/davinci/vpif_capture.c:940: warning: No 
> description found for parameter 'fmt'
> drivers/media/platform/davinci/vpif_capture.c:940: warning: Excess 
> function parameter 'index' description in 'vpif_enum_fmt_vid_cap'
> drivers/media/platform/davinci/vpif_capture.c:1750: warning: No 
> description found for parameter 'dev'
>
> Fix them.
>
> Signed-off-by: Mauro Carvalho Chehab <mche...@s-opensource.com>
> ---
>  drivers/media/platform/davinci/vpif_capture.c | 27 
> ++-
>  drivers/media/platform/davinci/vpif_display.c | 16 
>  2 files changed, 22 insertions(+), 21 deletions(-)
>

Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Regards,
--Prabhakar Lad

Re: [PATCH 06/12] media: vpif: don't generate a kernel-doc warning on a constant

[Lad, Prabhakar]

On Wed, Nov 29, 2017 at 10:46 AM, Mauro Carvalho Chehab
<mche...@s-opensource.com> wrote:
> Constants documentation is not supported by kernel-doc markups.
> So, change the comment label to avoid this warning:
> drivers/media/platform/davinci/vpif.c:54: warning: cannot understand 
> function prototype: 'const struct vpif_channel_config_params vpif_ch_params[] 
> = '
>
> Signed-off-by: Mauro Carvalho Chehab <mche...@s-opensource.com>
> ---
>  drivers/media/platform/davinci/vpif.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>

Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Regards,
--Prabhakar Lad

Re: [PATCH v2 12/26] media: davinci: fix a debug printk

[Lad, Prabhakar]

On Wed, Nov 1, 2017 at 9:05 PM, Mauro Carvalho Chehab
<mche...@s-opensource.com> wrote:
> Two orthogonal changesets caused a breakage at a printk
> inside davinci. Changeset a2d17962c9ca
> ("[media] davinci: Switch from V4L2 OF to V4L2 fwnode")
> made davinci to use struct fwnode_handle instead of
> struct device_node. Changeset 68d9c47b1679
> ("media: Convert to using %pOF instead of full_name")
> changed the printk to not use ->full_name, but, instead,
> to rely on %pOF.
>
> With both patches applied, the Kernel will do the wrong
> thing, as warned by smatch:
> drivers/media/platform/davinci/vpif_capture.c:1399 vpif_async_bound() 
> error: '%pOF' expects argument of type 'struct device_node*', argument 5 has 
> type 'void*'
>
> So, change the logic to actually print the device name
> that was obtained before the print logic.
>
> Fixes: 68d9c47b1679 ("media: Convert to using %pOF instead of full_name")
> Fixes: a2d17962c9ca ("[media] davinci: Switch from V4L2 OF to V4L2 fwnode")
> Signed-off-by: Mauro Carvalho Chehab <mche...@s-opensource.com>
> ---
>  drivers/media/platform/davinci/vpif_capture.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>

Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Cheers,
--Prabhakar Lad

Re: [PATCH 3/3] [media] DaVinci-VPBE-Display: Adjust 12 checks for null pointers

[Lad, Prabhakar]

On Fri, Sep 8, 2017 at 1:33 PM, SF Markus Elfring
<elfr...@users.sourceforge.net> wrote:
> From: Markus Elfring <elfr...@users.sourceforge.net>
> Date: Fri, 8 Sep 2017 14:00:20 +0200
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> The script “checkpatch.pl” pointed information out like the following.
>
> Comparison to NULL could be written …
>
> Thus fix the affected source code places.
>
> Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net>

Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Cheers,
--Prabhakar Lad

Re: [PATCH 2/3] [media] DaVinci-VPBE-Display: Improve a size determination in two functions

[Lad, Prabhakar]

On Fri, Sep 8, 2017 at 1:32 PM, SF Markus Elfring
<elfr...@users.sourceforge.net> wrote:
> From: Markus Elfring <elfr...@users.sourceforge.net>
> Date: Fri, 8 Sep 2017 10:50:32 +0200
>
> Replace the specification of data structures by pointer dereferences
> as the parameter for the operator "sizeof" to make the corresponding size
> determination a bit safer according to the Linux coding style convention.
>
> Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net>

Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Cheers,
--Prabhakar Lad

Re: [PATCH 1/3] [media] DaVinci-VPBE-Display: Delete an error message for a failed memory allocation in init_vpbe_layer()

[Lad, Prabhakar]

On Fri, Sep 8, 2017 at 1:31 PM, SF Markus Elfring
<elfr...@users.sourceforge.net> wrote:
>
> From: Markus Elfring <elfr...@users.sourceforge.net>
> Date: Thu, 7 Sep 2017 22:37:16 +0200
>
> Omit an extra message for a memory allocation failure in this function.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net>

Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Cheers,
--Prabhakar Lad

Re: [PATCH 2/3] [media] davinci: constify platform_device_id

[Lad, Prabhakar]

On Tue, Aug 15, 2017 at 12:23 PM, Arvind Yadav
<arvind.yadav...@gmail.com> wrote:
>
> platform_device_id are not supposed to change at runtime. All functions
> working with platform_device_id provided by 
> work with const platform_device_id. So mark the non-const structs as
> const.
>
> Signed-off-by: Arvind Yadav <arvind.yadav...@gmail.com>

Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Cheers,
--Prabhakar Lad

[v4] media: platform: davinci: return -EINVAL for VPFE_CMD_S_CCDC_RAW_PARAMS ioctl

[Lad, Prabhakar]

this patch makes sure VPFE_CMD_S_CCDC_RAW_PARAMS ioctl no longer works
for vpfe_capture driver with a minimal patch suitable for backporting.

- This ioctl was never in public api and was only defined in kernel header.
- The function set_params constantly mixes up pointers and phys_addr_t
  numbers.
- This is part of a 'VPFE_CMD_S_CCDC_RAW_PARAMS' ioctl command that is
  described as an 'experimental ioctl that will change in future kernels'.
- The code to allocate the table never gets called after we copy_from_user
  the user input over the kernel settings, and then compare them
  for inequality.
- We then go on to use an address provided by user space as both the
  __user pointer for input and pass it through phys_to_virt to come up
  with a kernel pointer to copy the data to. This looks like a trivially
  exploitable root hole.

Due to these reasons we make sure this ioctl now returns -EINVAL and backport
this patch as far as possible.

Fixes: 5f15fbb68fd7 ("V4L/DVB (12251): v4l: dm644x ccdc module for vpfe capture 
driver")
Signed-off-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>
---
 drivers/media/platform/davinci/vpfe_capture.c | 22 ++
 1 file changed, 2 insertions(+), 20 deletions(-)

diff --git a/drivers/media/platform/davinci/vpfe_capture.c 
b/drivers/media/platform/davinci/vpfe_capture.c
index e3fe3e0..1831bf5 100644
--- a/drivers/media/platform/davinci/vpfe_capture.c
+++ b/drivers/media/platform/davinci/vpfe_capture.c
@@ -1719,27 +1719,9 @@ static long vpfe_param_handler(struct file *file, void 
*priv,
 
switch (cmd) {
case VPFE_CMD_S_CCDC_RAW_PARAMS:
+   ret = -EINVAL;
v4l2_warn(_dev->v4l2_dev,
- "VPFE_CMD_S_CCDC_RAW_PARAMS: experimental ioctl\n");
-   if (ccdc_dev->hw_ops.set_params) {
-   ret = ccdc_dev->hw_ops.set_params(param);
-   if (ret) {
-   v4l2_dbg(1, debug, _dev->v4l2_dev,
-   "Error setting parameters in CCDC\n");
-   goto unlock_out;
-   }
-   ret = vpfe_get_ccdc_image_format(vpfe_dev,
-_dev->fmt);
-   if (ret < 0) {
-   v4l2_dbg(1, debug, _dev->v4l2_dev,
-   "Invalid image format at CCDC\n");
-   goto unlock_out;
-   }
-   } else {
-   ret = -EINVAL;
-   v4l2_dbg(1, debug, _dev->v4l2_dev,
-   "VPFE_CMD_S_CCDC_RAW_PARAMS not supported\n");
-   }
+   "VPFE_CMD_S_CCDC_RAW_PARAMS not supported\n");
break;
default:
ret = -ENOTTY;
-- 
2.7.4

Re: [PATCH] media: Convert to using %pOF instead of full_name

[Lad, Prabhakar]

On Tue, Jul 18, 2017 at 10:43 PM, Rob Herring <r...@kernel.org> wrote:
> Now that we have a custom printf format specifier, convert users of
> full_name to use %pOF instead. This is preparation to remove storing
> of the full path string for each node.
>
> Signed-off-by: Rob Herring <r...@kernel.org>
> Cc: Kyungmin Park <kyungmin.p...@samsung.com>
> Cc: Andrzej Hajda <a.ha...@samsung.com>
> Cc: Mauro Carvalho Chehab <mche...@kernel.org>
> Cc: "Lad, Prabhakar" <prabhakar.cse...@gmail.com>
> Cc: Songjun Wu <songjun...@microchip.com>
> Cc: Sylwester Nawrocki <s.nawro...@samsung.com>
> Cc: Kukjin Kim <kg...@kernel.org>
> Cc: Krzysztof Kozlowski <k...@kernel.org>
> Cc: Javier Martinez Canillas <jav...@osg.samsung.com>
> Cc: Minghsiu Tsai <minghsiu.t...@mediatek.com>
> Cc: Houlong Wei <houlong@mediatek.com>
> Cc: Andrew-CT Chen <andrew-ct.c...@mediatek.com>
> Cc: Matthias Brugger <matthias@gmail.com>
> Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> Cc: "Niklas Söderlund" <niklas.soderl...@ragnatech.se>
> Cc: Guennadi Liakhovetski <g.liakhovet...@gmx.de>
> Cc: Hyun Kwon <hyun.k...@xilinx.com>
> Cc: Michal Simek <michal.si...@xilinx.com>
> Cc: "Sören Brinkmann" <soren.brinkm...@xilinx.com>
> Cc: linux-media@vger.kernel.org
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-samsung-...@vger.kernel.org
> Cc: linux-media...@lists.infradead.org
> Cc: linux-renesas-...@vger.kernel.org
> ---
>  drivers/media/platform/am437x/am437x-vpfe.c|  4 +-
>  drivers/media/platform/davinci/vpif_capture.c  | 16 

For above:

Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Cheers,
--Prabhakar Lad

[v3 2/2] media: platform: davinci: drop VPFE_CMD_S_CCDC_RAW_PARAMS

[Lad, Prabhakar]

drop VPFE_CMD_S_CCDC_RAW_PARAMS ioctl from dm355/dm644x following reasons:

- This ioctl was never in public api and was only defined in kernel header.
- The function set_params constantly mixes up pointers and phys_addr_t
  numbers.
- This is part of a 'VPFE_CMD_S_CCDC_RAW_PARAMS' ioctl command that is
  described as an 'experimental ioctl that will change in future kernels'.
- The code to allocate the table never gets called after we copy_from_user
  the user input over the kernel settings, and then compare them
  for inequality.
- We then go on to use an address provided by user space as both the
  __user pointer for input and pass it through phys_to_virt to come up
  with a kernel pointer to copy the data to. This looks like a trivially
  exploitable root hole.

Signed-off-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>
---
 drivers/media/platform/davinci/ccdc_hw_device.h |  10 --
 drivers/media/platform/davinci/dm355_ccdc.c |  92 +--
 drivers/media/platform/davinci/dm644x_ccdc.c| 151 +---
 drivers/media/platform/davinci/vpfe_capture.c   |  75 
 include/media/davinci/dm644x_ccdc.h |  12 --
 include/media/davinci/vpfe_capture.h|  10 --
 6 files changed, 4 insertions(+), 346 deletions(-)

diff --git a/drivers/media/platform/davinci/ccdc_hw_device.h 
b/drivers/media/platform/davinci/ccdc_hw_device.h
index 8f6688a..f1b5210 100644
--- a/drivers/media/platform/davinci/ccdc_hw_device.h
+++ b/drivers/media/platform/davinci/ccdc_hw_device.h
@@ -42,16 +42,6 @@ struct ccdc_hw_ops {
int (*set_hw_if_params) (struct vpfe_hw_if_param *param);
/* get interface parameters */
int (*get_hw_if_params) (struct vpfe_hw_if_param *param);
-   /*
-* Pointer to function to set parameters. Used
-* for implementing VPFE_S_CCDC_PARAMS
-*/
-   int (*set_params) (void *params);
-   /*
-* Pointer to function to get parameter. Used
-* for implementing VPFE_G_CCDC_PARAMS
-*/
-   int (*get_params) (void *params);
/* Pointer to function to configure ccdc */
int (*configure) (void);
 
diff --git a/drivers/media/platform/davinci/dm355_ccdc.c 
b/drivers/media/platform/davinci/dm355_ccdc.c
index 73db166..6d492dc 100644
--- a/drivers/media/platform/davinci/dm355_ccdc.c
+++ b/drivers/media/platform/davinci/dm355_ccdc.c
@@ -17,12 +17,7 @@
  * This module is for configuring DM355 CCD controller of VPFE to capture
  * Raw yuv or Bayer RGB data from a decoder. CCDC has several modules
  * such as Defect Pixel Correction, Color Space Conversion etc to
- * pre-process the Bayer RGB data, before writing it to SDRAM. This
- * module also allows application to configure individual
- * module parameters through VPFE_CMD_S_CCDC_RAW_PARAMS IOCTL.
- * To do so, application include dm355_ccdc.h and vpfe_capture.h header
- * files. The setparams() API is called by vpfe_capture driver
- * to configure module parameters
+ * pre-process the Bayer RGB data, before writing it to SDRAM.
  *
  * TODO: 1) Raw bayer parameter settings and bayer capture
  *  2) Split module parameter structure to module specific ioctl structs
@@ -260,90 +255,6 @@ static void ccdc_setwin(struct v4l2_rect *image_win,
dev_dbg(ccdc_cfg.dev, "\nEnd of ccdc_setwin...");
 }
 
-static int validate_ccdc_param(struct ccdc_config_params_raw *ccdcparam)
-{
-   if (ccdcparam->datasft < CCDC_DATA_NO_SHIFT ||
-   ccdcparam->datasft > CCDC_DATA_SHIFT_6BIT) {
-   dev_dbg(ccdc_cfg.dev, "Invalid value of data shift\n");
-   return -EINVAL;
-   }
-
-   if (ccdcparam->mfilt1 < CCDC_NO_MEDIAN_FILTER1 ||
-   ccdcparam->mfilt1 > CCDC_MEDIAN_FILTER1) {
-   dev_dbg(ccdc_cfg.dev, "Invalid value of median filter1\n");
-   return -EINVAL;
-   }
-
-   if (ccdcparam->mfilt2 < CCDC_NO_MEDIAN_FILTER2 ||
-   ccdcparam->mfilt2 > CCDC_MEDIAN_FILTER2) {
-   dev_dbg(ccdc_cfg.dev, "Invalid value of median filter2\n");
-   return -EINVAL;
-   }
-
-   if ((ccdcparam->med_filt_thres < 0) ||
-  (ccdcparam->med_filt_thres > CCDC_MED_FILT_THRESH)) {
-   dev_dbg(ccdc_cfg.dev,
-   "Invalid value of median filter threshold\n");
-   return -EINVAL;
-   }
-
-   if (ccdcparam->data_sz < CCDC_DATA_16BITS ||
-   ccdcparam->data_sz > CCDC_DATA_8BITS) {
-   dev_dbg(ccdc_cfg.dev, "Invalid value of data size\n");
-   return -EINVAL;
-   }
-
-   if (ccdcparam->alaw.enable) {
-   if (ccdcparam->alaw.gamma_wd < CCDC_GAMMA_BITS_13_4 ||
-   ccdcparam->alaw.gamma_wd > CCDC_GAMMA_BITS_09_0) {
-   dev_dbg(ccdc_cfg.dev, &quo

[v3 0/2] media: platform: davinci: remove VPFE_CMD_S_CCDC_RAW_PARAMS IOCTL

[Lad, Prabhakar]

This patch series drops VPFE_CMD_S_CCDC_RAW_PARAMS ioctl, from davicni
vpfe_capture driver because of following reasons:

- The function constantly mixes up pointers and phys_addr_t numbers
- This is part of a 'VPFE_CMD_S_CCDC_RAW_PARAMS' ioctl command that is
  described as an 'experimental ioctl that will change in future kernels',
  but if we have users that probably won't happen.
- The code to allocate the table never gets called after we copy_from_user
  the user input over the kernel settings, and then compare them
  for inequality.
- We then go on to use an address provided by user space as both the
  __user pointer for input and pass it through phys_to_virt to come up
  with a kernel pointer to copy the data to. This looks like a trivially
  exploitable root hole.


As discussed at [1], there wouldn’t be any possible users of
the VPFE_CMD_S_CCDC_RAW_PARAMS IOCTL, but if someone complains
we might end up reverting the removal and fix it differently.

[1] https://patchwork.kernel.org/patch/9779385/

Note: Patch 0001 is intended to apply for backports so as the reason
minimalistic changes have been done since the ioctl was added in
kernel 2.6.32 [2] and applying too many changes would produce conflicts,
just applying this patch would produce build warning of unused functions.

[2] commit 5f15fbb68fd7 ("V4L/DVB (12251): v4l: dm644x ccdc module for vpfe 
capture driver")

Lad, Prabhakar (2):
  media: platform: davinci: prepare for removal of
VPFE_CMD_S_CCDC_RAW_PARAMS ioctl
  media: platform: davinci: drop VPFE_CMD_S_CCDC_RAW_PARAMS

 drivers/media/platform/davinci/ccdc_hw_device.h |  10 --
 drivers/media/platform/davinci/dm355_ccdc.c |  92 +--
 drivers/media/platform/davinci/dm644x_ccdc.c| 151 +---
 drivers/media/platform/davinci/vpfe_capture.c   |  93 ---
 include/media/davinci/dm644x_ccdc.h |  12 --
 include/media/davinci/vpfe_capture.h|  10 --
 6 files changed, 4 insertions(+), 364 deletions(-)

-- 
2.7.4

[v3 1/2] media: platform: davinci: prepare for removal of VPFE_CMD_S_CCDC_RAW_PARAMS ioctl

[Lad, Prabhakar]

preparing for removal of VPFE_CMD_S_CCDC_RAW_PARAMS ioctl from
davicni vpfe_capture driver because of following reasons:

- This ioctl was never in public api and was only defined in kernel header.
- The function set_params constantly mixes up pointers and phys_addr_t
  numbers.
- This is part of a 'VPFE_CMD_S_CCDC_RAW_PARAMS' ioctl command that is
  described as an 'experimental ioctl that will change in future kernels'.
- The code to allocate the table never gets called after we copy_from_user
  the user input over the kernel settings, and then compare them
  for inequality.
- We then go on to use an address provided by user space as both the
  __user pointer for input and pass it through phys_to_virt to come up
  with a kernel pointer to copy the data to. This looks like a trivially
  exploitable root hole.

Fixes: 5f15fbb68fd7 ("V4L/DVB (12251): v4l: dm644x ccdc module for vpfe capture 
driver")
Signed-off-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>
---
 drivers/media/platform/davinci/vpfe_capture.c | 22 ++
 1 file changed, 2 insertions(+), 20 deletions(-)

diff --git a/drivers/media/platform/davinci/vpfe_capture.c 
b/drivers/media/platform/davinci/vpfe_capture.c
index e3fe3e0..1831bf5 100644
--- a/drivers/media/platform/davinci/vpfe_capture.c
+++ b/drivers/media/platform/davinci/vpfe_capture.c
@@ -1719,27 +1719,9 @@ static long vpfe_param_handler(struct file *file, void 
*priv,
 
switch (cmd) {
case VPFE_CMD_S_CCDC_RAW_PARAMS:
+   ret = -EINVAL;
v4l2_warn(_dev->v4l2_dev,
- "VPFE_CMD_S_CCDC_RAW_PARAMS: experimental ioctl\n");
-   if (ccdc_dev->hw_ops.set_params) {
-   ret = ccdc_dev->hw_ops.set_params(param);
-   if (ret) {
-   v4l2_dbg(1, debug, _dev->v4l2_dev,
-   "Error setting parameters in CCDC\n");
-   goto unlock_out;
-   }
-   ret = vpfe_get_ccdc_image_format(vpfe_dev,
-_dev->fmt);
-   if (ret < 0) {
-   v4l2_dbg(1, debug, _dev->v4l2_dev,
-   "Invalid image format at CCDC\n");
-   goto unlock_out;
-   }
-   } else {
-   ret = -EINVAL;
-   v4l2_dbg(1, debug, _dev->v4l2_dev,
-   "VPFE_CMD_S_CCDC_RAW_PARAMS not supported\n");
-   }
+   "VPFE_CMD_S_CCDC_RAW_PARAMS not supported\n");
break;
default:
ret = -ENOTTY;
-- 
2.7.4

[PATCH v2 0/2] media: platform: davinci: remove VPFE_CMD_S_CCDC_RAW_PARAMS IOCTL

[Lad, Prabhakar]

This patch series drops VPFE_CMD_S_CCDC_RAW_PARAMS ioctl, from davicni
vpfe_capture driver because of following reasons:

- The function constantly mixes up pointers and phys_addr_t numbers
- This is part of a 'VPFE_CMD_S_CCDC_RAW_PARAMS' ioctl command that is
  described as an 'experimental ioctl that will change in future kernels',
  but if we have users that probably won't happen.
- The code to allocate the table never gets called after we copy_from_user
  the user input over the kernel settings, and then compare them
  for inequality.
- We then go on to use an address provided by user space as both the
  __user pointer for input and pass it through phys_to_virt to come up
  with a kernel pointer to copy the data to. This looks like a trivially
  exploitable root hole.


As discussed at [1], there wouldn’t be any possible users of
the VPFE_CMD_S_CCDC_RAW_PARAMS IOCTL, but if someone complains
we might end up reverting the removal and fix it differently.

[1] https://patchwork.kernel.org/patch/9779385/

Note: Patch 0001 is intended to apply for backports so as the reason
minimalistic changes have been done since the ioctl was added in
kernel 2.6.32 [2] and applying too many changes would produce conflicts,
just applying this patch would produce build warning of unused functions.

[2] commit 5f15fbb68fd7 ("V4L/DVB (12251): v4l: dm644x ccdc module for vpfe 
capture driver")

Lad, Prabhakar (2):
  media: platform: davinci: prepare for removal of
VPFE_CMD_S_CCDC_RAW_PARAMS ioctl
  media: platform: davinci: drop VPFE_CMD_S_CCDC_RAW_PARAMS

 drivers/media/platform/davinci/ccdc_hw_device.h |  10 --
 drivers/media/platform/davinci/dm355_ccdc.c |  92 +--
 drivers/media/platform/davinci/dm644x_ccdc.c| 151 +---
 drivers/media/platform/davinci/vpfe_capture.c   |  93 ---
 include/media/davinci/dm644x_ccdc.h |  12 --
 include/media/davinci/vpfe_capture.h|  10 --
 6 files changed, 4 insertions(+), 364 deletions(-)

-- 
2.7.4

[PATCH v2 1/2] media: platform: davinci: prepare for removal of VPFE_CMD_S_CCDC_RAW_PARAMS ioctl

[Lad, Prabhakar]

preparing for removal of VPFE_CMD_S_CCDC_RAW_PARAMS ioctl having
minimalistic code changes so as it can be applied for backports.

Signed-off-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>
---
 drivers/media/platform/davinci/vpfe_capture.c | 22 ++
 1 file changed, 2 insertions(+), 20 deletions(-)

diff --git a/drivers/media/platform/davinci/vpfe_capture.c 
b/drivers/media/platform/davinci/vpfe_capture.c
index e3fe3e0..1831bf5 100644
--- a/drivers/media/platform/davinci/vpfe_capture.c
+++ b/drivers/media/platform/davinci/vpfe_capture.c
@@ -1719,27 +1719,9 @@ static long vpfe_param_handler(struct file *file, void 
*priv,
 
switch (cmd) {
case VPFE_CMD_S_CCDC_RAW_PARAMS:
+   ret = -EINVAL;
v4l2_warn(_dev->v4l2_dev,
- "VPFE_CMD_S_CCDC_RAW_PARAMS: experimental ioctl\n");
-   if (ccdc_dev->hw_ops.set_params) {
-   ret = ccdc_dev->hw_ops.set_params(param);
-   if (ret) {
-   v4l2_dbg(1, debug, _dev->v4l2_dev,
-   "Error setting parameters in CCDC\n");
-   goto unlock_out;
-   }
-   ret = vpfe_get_ccdc_image_format(vpfe_dev,
-_dev->fmt);
-   if (ret < 0) {
-   v4l2_dbg(1, debug, _dev->v4l2_dev,
-   "Invalid image format at CCDC\n");
-   goto unlock_out;
-   }
-   } else {
-   ret = -EINVAL;
-   v4l2_dbg(1, debug, _dev->v4l2_dev,
-   "VPFE_CMD_S_CCDC_RAW_PARAMS not supported\n");
-   }
+   "VPFE_CMD_S_CCDC_RAW_PARAMS not supported\n");
break;
default:
ret = -ENOTTY;
-- 
2.7.4

[PATCH v2 2/2] media: platform: davinci: drop VPFE_CMD_S_CCDC_RAW_PARAMS

[Lad, Prabhakar]

For dm355 and dm644x the vpfe driver provided a ioctl to
configure the raw bayer config using a IOCTL, but since
the code was not properly implemented and aswell the
IOCTL was marked as 'experimental ioctl that will change
in future kernels', dropping this IOCTL.

Signed-off-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>
---
 drivers/media/platform/davinci/ccdc_hw_device.h |  10 --
 drivers/media/platform/davinci/dm355_ccdc.c |  92 +--
 drivers/media/platform/davinci/dm644x_ccdc.c| 151 +---
 drivers/media/platform/davinci/vpfe_capture.c   |  75 
 include/media/davinci/dm644x_ccdc.h |  12 --
 include/media/davinci/vpfe_capture.h|  10 --
 6 files changed, 4 insertions(+), 346 deletions(-)

diff --git a/drivers/media/platform/davinci/ccdc_hw_device.h 
b/drivers/media/platform/davinci/ccdc_hw_device.h
index 8f6688a..f1b5210 100644
--- a/drivers/media/platform/davinci/ccdc_hw_device.h
+++ b/drivers/media/platform/davinci/ccdc_hw_device.h
@@ -42,16 +42,6 @@ struct ccdc_hw_ops {
int (*set_hw_if_params) (struct vpfe_hw_if_param *param);
/* get interface parameters */
int (*get_hw_if_params) (struct vpfe_hw_if_param *param);
-   /*
-* Pointer to function to set parameters. Used
-* for implementing VPFE_S_CCDC_PARAMS
-*/
-   int (*set_params) (void *params);
-   /*
-* Pointer to function to get parameter. Used
-* for implementing VPFE_G_CCDC_PARAMS
-*/
-   int (*get_params) (void *params);
/* Pointer to function to configure ccdc */
int (*configure) (void);
 
diff --git a/drivers/media/platform/davinci/dm355_ccdc.c 
b/drivers/media/platform/davinci/dm355_ccdc.c
index 73db166..6d492dc 100644
--- a/drivers/media/platform/davinci/dm355_ccdc.c
+++ b/drivers/media/platform/davinci/dm355_ccdc.c
@@ -17,12 +17,7 @@
  * This module is for configuring DM355 CCD controller of VPFE to capture
  * Raw yuv or Bayer RGB data from a decoder. CCDC has several modules
  * such as Defect Pixel Correction, Color Space Conversion etc to
- * pre-process the Bayer RGB data, before writing it to SDRAM. This
- * module also allows application to configure individual
- * module parameters through VPFE_CMD_S_CCDC_RAW_PARAMS IOCTL.
- * To do so, application include dm355_ccdc.h and vpfe_capture.h header
- * files. The setparams() API is called by vpfe_capture driver
- * to configure module parameters
+ * pre-process the Bayer RGB data, before writing it to SDRAM.
  *
  * TODO: 1) Raw bayer parameter settings and bayer capture
  *  2) Split module parameter structure to module specific ioctl structs
@@ -260,90 +255,6 @@ static void ccdc_setwin(struct v4l2_rect *image_win,
dev_dbg(ccdc_cfg.dev, "\nEnd of ccdc_setwin...");
 }
 
-static int validate_ccdc_param(struct ccdc_config_params_raw *ccdcparam)
-{
-   if (ccdcparam->datasft < CCDC_DATA_NO_SHIFT ||
-   ccdcparam->datasft > CCDC_DATA_SHIFT_6BIT) {
-   dev_dbg(ccdc_cfg.dev, "Invalid value of data shift\n");
-   return -EINVAL;
-   }
-
-   if (ccdcparam->mfilt1 < CCDC_NO_MEDIAN_FILTER1 ||
-   ccdcparam->mfilt1 > CCDC_MEDIAN_FILTER1) {
-   dev_dbg(ccdc_cfg.dev, "Invalid value of median filter1\n");
-   return -EINVAL;
-   }
-
-   if (ccdcparam->mfilt2 < CCDC_NO_MEDIAN_FILTER2 ||
-   ccdcparam->mfilt2 > CCDC_MEDIAN_FILTER2) {
-   dev_dbg(ccdc_cfg.dev, "Invalid value of median filter2\n");
-   return -EINVAL;
-   }
-
-   if ((ccdcparam->med_filt_thres < 0) ||
-  (ccdcparam->med_filt_thres > CCDC_MED_FILT_THRESH)) {
-   dev_dbg(ccdc_cfg.dev,
-   "Invalid value of median filter threshold\n");
-   return -EINVAL;
-   }
-
-   if (ccdcparam->data_sz < CCDC_DATA_16BITS ||
-   ccdcparam->data_sz > CCDC_DATA_8BITS) {
-   dev_dbg(ccdc_cfg.dev, "Invalid value of data size\n");
-   return -EINVAL;
-   }
-
-   if (ccdcparam->alaw.enable) {
-   if (ccdcparam->alaw.gamma_wd < CCDC_GAMMA_BITS_13_4 ||
-   ccdcparam->alaw.gamma_wd > CCDC_GAMMA_BITS_09_0) {
-   dev_dbg(ccdc_cfg.dev, "Invalid value of ALAW\n");
-   return -EINVAL;
-   }
-   }
-
-   if (ccdcparam->blk_clamp.b_clamp_enable) {
-   if (ccdcparam->blk_clamp.sample_pixel < CCDC_SAMPLE_1PIXELS ||
-   ccdcparam->blk_clamp.sample_pixel > CCDC_SAMPLE_16PIXELS) {
-   dev_dbg(ccdc_cfg.dev,
-   "Invalid value of sample pixel\n");
-   return -EINVAL;
-   }
-   if 

Re: [PATCH] media: platform: davinci: drop VPFE_CMD_S_CCDC_RAW_PARAMS

[Lad, Prabhakar]

Hi Arnd,

On Fri, Jun 30, 2017 at 11:03 AM, Arnd Bergmann <a...@arndb.de> wrote:
> On Fri, Jun 30, 2017 at 11:32 AM, Prabhakar <prabhakar.cse...@gmail.com> 
> wrote:
>> From: "Lad, Prabhakar" <prabhakar.cse...@gmail.com>
>>
>> For dm355 and dm644x the vpfe driver provided a ioctl to
>> configure the raw bayer config using a IOCTL, but since
>> the code was not properly implemented and aswell the
>> IOCTL was marked as 'experimental ioctl that will change
>> in future kernels', dropping this IOCTL.
>>
>> Signed-off-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>
>> ---
>> As discussed at [1], there wouldn’t be any possible users of
>> the VPFE_CMD_S_CCDC_RAW_PARAMS IOCTL, but if someone complains
>> we might end up reverting the removal and fix it differently.
>>
>> Note: This patch is on top of [1].
>>
>> [1] https://patchwork.kernel.org/patch/9779385/
>
> Acked-by: Arnd Bergmann <a...@arndb.de>
>
Thanks for the Ack.

> I think it would be good to backport one or both of the patches to
> stable kernels, to close the potential risk of someone abusing the
> ioctl.

I think just your patch should be suffice to the stable kernel,
blocking this IOCTL.

Cheers,
--Prabhakar Lad

Re: [PATCH] [media] media/platform: add const to v4l2_file_operations structures

[Lad, Prabhakar]

Hi,

Thanks for the patch.

On Thu, Jun 29, 2017 at 9:51 AM, Bhumika Goyal <bhumi...@gmail.com> wrote:
> Declare v4l2_file_operations structures as const as they are only stored
> in the fops field of video_device structures. This field is of type
> const, so declare v4l2_file_operations structures with similar properties
> as const.
>
> Cross compiled bfin_capture.o for blackfin arch. vpbe_display.o file did
> not cross compile for arm. Could not find any architecture matching the
> configuraion symbol for fsl-viu.c file.
>
s/configuraion/configuration

Ideally the above statement should go below  --- (after the signoff) so that
its not the part of commit message.

> Signed-off-by: Bhumika Goyal <bhumi...@gmail.com>
> ---
>  drivers/media/platform/davinci/vpbe_display.c  | 2 +-
>  drivers/media/platform/davinci/vpif_capture.c  | 2 +-

For above:
Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Cheers,
--Prabhakar Lad

Re: [PATCH] [media] davinci/dm644x: work around ccdc_update_raw_params trainwreck

[Lad, Prabhakar]

Hi Arnd,

On Tue, Jun 27, 2017 at 12:08 PM, Arnd Bergmann <a...@arndb.de> wrote:
> On Tue, Jun 27, 2017 at 12:13 PM, Sekhar Nori <nsek...@ti.com> wrote:
>> On Tuesday 20 June 2017 06:36 PM, Lad, Prabhakar wrote:
>>> Hi Arnd,
>>>
>>> Thanks for the patch.
>>>
>>> On Fri, Jun 9, 2017 at 10:36 PM, Arnd Bergmann <a...@arndb.de> wrote:
>>>> Now that the davinci drivers can be enabled in compile tests on other
>>>> architectures, I ran into this warning on a 64-bit build:
>>>>
>>>> drivers/media/platform/davinci/dm644x_ccdc.c: In function 
>>>> 'ccdc_update_raw_params':
>>>> drivers/media/platform/davinci/dm644x_ccdc.c:279:7: error: cast to pointer 
>>>> from integer of different size [-Werror=int-to-pointer-cast]
>>>>
>>>> While that looks fairly harmless (it would be fine on 32-bit), it was
>>>> just the tip of the iceberg:
>>>>
>>>> - The function constantly mixes up pointers and phys_addr_t numbers
>>>> - This is part of a 'VPFE_CMD_S_CCDC_RAW_PARAMS' ioctl command that is
>>>>   described as an 'experimental ioctl that will change in future kernels',
>>>>   but if we have users that probably won't happen.
>>>> - The code to allocate the table never gets called after we copy_from_user
>>>>   the user input over the kernel settings, and then compare them
>>>>   for inequality.
>>>> - We then go on to use an address provided by user space as both the
>>>>   __user pointer for input and pass it through phys_to_virt to come up
>>>>   with a kernel pointer to copy the data to. This looks like a trivially
>>>>   exploitable root hole.
>>>>
>>>> This patch disables all the obviously broken code, by zeroing out the
>>>> sensitive data provided by user space. I also fix the type confusion
>>>> here. If we think the ioctl has no stable users, we could consider
>>>> just removing it instead.
>>>>
>>> I suspect there shouldn’t  be possible users of this IOCTL, better of  
>>> removing
>>> the IOCTL itself.
>>>
>>> Sekhar your call, as the latest PSP releases for 644x use the media
>>> controller framework.
>>
>> I do not have any personal experience with anyone using this support
>> with latest kernels. I too am okay with removing the broken support.
>
> Ok, I think that would be good. Can one of you create that patch?
> Note that we have two implementations of the ioctl, with different
> data structures, depending on the specific hardware.
>
I have posted a patch on top of yours.

Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

>> Since the header file that defines the ioctl is not in include/uapi/*, I
>> guess it cannot be considered stable userspace ABI? Also, there are
>> enough warnings about instability thrown in the comments surrounding the
>> ioctl in include/media/davinci/vpfe_capture.h.
>
> This is not relevant really. The only thing that counts is whether there
> is existing user space that has active users who complain if it breaks.
>
> If you think nobody is using it, that is more important than code
> comments or the location of the header file, but if someone complains
> later anyway, we may end up reverting the removal and fix it differently.
>
Agreed.

Cheers,
--Prabhakar Lad

Re: [PATCH] [media] davinci/dm644x: work around ccdc_update_raw_params trainwreck

[Lad, Prabhakar]

Hi Arnd,

Thanks for the patch.

On Fri, Jun 9, 2017 at 10:36 PM, Arnd Bergmann  wrote:
> Now that the davinci drivers can be enabled in compile tests on other
> architectures, I ran into this warning on a 64-bit build:
>
> drivers/media/platform/davinci/dm644x_ccdc.c: In function 
> 'ccdc_update_raw_params':
> drivers/media/platform/davinci/dm644x_ccdc.c:279:7: error: cast to pointer 
> from integer of different size [-Werror=int-to-pointer-cast]
>
> While that looks fairly harmless (it would be fine on 32-bit), it was
> just the tip of the iceberg:
>
> - The function constantly mixes up pointers and phys_addr_t numbers
> - This is part of a 'VPFE_CMD_S_CCDC_RAW_PARAMS' ioctl command that is
>   described as an 'experimental ioctl that will change in future kernels',
>   but if we have users that probably won't happen.
> - The code to allocate the table never gets called after we copy_from_user
>   the user input over the kernel settings, and then compare them
>   for inequality.
> - We then go on to use an address provided by user space as both the
>   __user pointer for input and pass it through phys_to_virt to come up
>   with a kernel pointer to copy the data to. This looks like a trivially
>   exploitable root hole.
>
> This patch disables all the obviously broken code, by zeroing out the
> sensitive data provided by user space. I also fix the type confusion
> here. If we think the ioctl has no stable users, we could consider
> just removing it instead.
>
I suspect there shouldn’t  be possible users of this IOCTL, better of  removing
the IOCTL itself.

Sekhar your call, as the latest PSP releases for 644x use the media
controller framework.

Cheers,
--Prabhakar Lad

Re: [PATCH v2] [media] davinci: vpif: adaptions for DT support

[Lad, Prabhakar]

On Fri, Jun 9, 2017 at 5:10 PM, Kevin Hilman <khil...@baylibre.com> wrote:
> The davinci VPIF is a single hardware block, but the existing driver
> is broken up into a common library (vpif.c), output (vpif_display.c) and
> intput (vpif_capture.c).
>
> When migrating to DT, to better model the hardware, and because
> registers, interrupts, etc. are all common,it was decided to
> have a single VPIF hardware node[1].
>
> Because davinci uses legacy, non-DT boot on several SoCs still, the
> platform_drivers need to remain.  But they are also needed in DT boot.
> Since there are no DT nodes for the display/capture parts in DT
> boot (there is a single node for the parent/common device) we need to
> create platform_devices somewhere to instansiate the platform_drivers.
>
> When VPIF display/capture are needed for a DT boot, the VPIF node
> will have endpoints defined for its subdevs.  Therefore, vpif_probe()
> checks for the presence of endpoints, and if detected manually creates
> the platform_devices for the display and capture platform_drivers.
>
> [1] Documentation/devicetree/bindings/media/ti,da850-vpif.txt
>
> Signed-off-by: Kevin Hilman <khil...@baylibre.com>

Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Cheers,
--Prabhakar Lad

Re: [PATCH v2 2/4] [media] davinci: vpif_capture: get subdevs from DT when available

[Lad, Prabhakar]

Hi Kevin,

Thanks for the patch.

On Fri, Jun 9, 2017 at 2:01 AM, Kevin Hilman  wrote:
> On Wed, Jun 7, 2017 at 11:29 PM, Hans Verkuil  wrote:
>> On 07/06/17 01:37, Kevin Hilman wrote:
>>> Enable  getting of subdevs from DT ports and endpoints.
>>>
>>> The _get_pdata() function was larely inspired by (i.e. stolen from)
>>> am437x-vpfe.c
>>>
>>> Signed-off-by: Kevin Hilman 
>>> ---
>>>  drivers/media/platform/davinci/vpif_capture.c | 126 
>>> +-
>>>  drivers/media/platform/davinci/vpif_display.c |   5 +

Can you make this as a separate patch.

>>>  include/media/davinci/vpif_types.h|   9 +-
>>>  3 files changed, 134 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/media/platform/davinci/vpif_capture.c 
>>> b/drivers/media/platform/davinci/vpif_capture.c
>>> index fc5c7622660c..b9d927d1e5a8 100644
>>> --- a/drivers/media/platform/davinci/vpif_capture.c
>>> +++ b/drivers/media/platform/davinci/vpif_capture.c
>>> @@ -22,6 +22,8 @@
>>>  #include 
>>>
>>>  #include 
>>> +#include 
>>
>> v4l2-of.h no longer exists, so this v2 is wrong. Unfortunately this patch has
>> already been merged in our master. I'm not sure how this could have slipped 
>> past
>> both my and Mauro's patch testing (and yours, for that matter).
>
> I have that file in the various trees I tested agains.
>
>> Can you fix this and post a patch on top of the media master that makes this
>> compile again?
>
> Sorry for the dumb question, but what tree are you referring to?  I
> tried the master branch of both [1] and [2] and both seem to have that
> include.
>
> Kevin
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media.git
> [2] git://linuxtv.org/mchehab/media-next.git

media master is https://git.linuxtv.org/media_tree.git/

Cheers,
--Prabhakar Lad

Re: [PATCH] davinci: vpif_capture: fix default pixel format for BT.656/BT.1120 video

[Lad, Prabhakar]

Hi Sekhar,

Thanks for the patch.

On Fri, May 26, 2017 at 11:55 AM, Sekhar Nori <nsek...@ti.com> wrote:
> For both BT.656 and BT.1120 video, the pixel format
> used by VPIF is Y/CbCr 4:2:2 in semi-planar format
> (Luma in one plane and Chroma in another). This
> corresponds to NV16 pixel format.
>
> This is documented in section 36.2.3 of OMAP-L138
> Technical Reference Manual, SPRUH77A.
>
> The VPIF driver incorrectly sets the default format
> to V4L2_PIX_FMT_YUV422P. Fix it.
>
> Reported-by: Alejandro Hernandez <ajhernan...@ti.com>
> Signed-off-by: Sekhar Nori <nsek...@ti.com>

Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Can you also post a similar patch for vpif_display as well ?

Cheers,
--Prabhakar Lad

Re: [PATCH] media: vpif: use a configurable i2c_adapter_id for vpif display

[Lad, Prabhakar]

Hi Bartosz,

Thanks for the patch.

On Thu, Feb 16, 2017 at 6:08 PM, Bartosz Golaszewski
<bgolaszew...@baylibre.com> wrote:
>
> The vpif display driver uses a static i2c adapter ID of 1 but on the
> da850-evm board in DT boot mode the i2c adapter ID is actually 0.
>
> Make the adapter ID configurable like it already is for vpif capture.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszew...@baylibre.com>
> Acked-by: Kevin Hilman <khil...@baylibre.com>
> ---
>  arch/arm/mach-davinci/board-da850-evm.c   | 1 +
>  drivers/media/platform/davinci/vpif_display.c | 2 +-
>  include/media/davinci/vpif_types.h| 1 +
>  3 files changed, 3 insertions(+), 1 deletion(-)
>

Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Cheers,
--Prabhakar Lad

Re: [PATCH] media: platform: constify vb2_ops structures

[Lad, Prabhakar]

Hi,

Thanks for the patch.

On Sat, Jan 21, 2017 at 9:29 AM, Bhumika Goyal  wrote:
> Declare vb2_ops structures as const as they are only stored in
> the ops field of a vb2_queue structure. This field is of type
> const, so vb2_ops structures having same properties can be made
> const too.
> Done using Coccinelle:
>
> @r1 disable optional_qualifier@
> identifier i;
> position p;
> @@
> static struct vb2_ops i@p={...};
>
> @ok1@
> identifier r1.i;
> position p;
> struct sta2x11_vip vip;
> struct vb2_queue q;
> @@
> (
> vip.vb_vidq.ops=@p
> |
> q.ops=@p
> )
>
> @bad@
> position p!={r1.p,ok1.p};
> identifier r1.i;
> @@
> i@p
>
> @depends on !bad disable optional_qualifier@
> identifier r1.i;
> @@
> +const
> struct vb2_ops i;
>
> Cross compiled the media/platform/blackfin/bfin_capture.o file for
> blackfin architecture.
>
> File size before:
>   text data bss dec hex filename
>   6776  176   069521b28 platform/blackfin/bfin_capture.o
>
The description doesnt match the changes. Can you please split the
patches separately one for vpif_capture.c and vpif_display.c,
one for mtk_mdp_m2m.c and lastly for pxa_camera.c .


Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 00/34] [media] DaVinci-Video Processing: Fine-tuning for several function implementations

[Lad, Prabhakar]

Hello,

Thanks for the patches.

On Wed, Oct 12, 2016 at 3:26 PM, SF Markus Elfring
<elfr...@users.sourceforge.net> wrote:
> From: Markus Elfring <elfr...@users.sourceforge.net>
> Date: Wed, 12 Oct 2016 16:20:02 +0200
>
> Several update suggestions were taken into account
> from static source code analysis.
>
> Markus Elfring (34):
>   Use kmalloc_array() in vpbe_initialize()
>   Delete two error messages for a failed memory allocation
>   Adjust 16 checks for null pointers
>   Combine substrings for four messages
>   Return an error code only as a constant in vpbe_probe()
>   Return an error code only by a single variable in vpbe_initialize()
>   Delete an unnecessary variable initialisation in vpbe_initialize()
>   Return the success indication only as a constant in vpbe_set_mode()
>   Reduce the scope for a variable in vpbe_set_default_output()
>   Check return value of a setup_if_config() call in vpbe_set_output()
>   Rename a jump label in vpbe_set_output()
>   Delete an unnecessary variable initialisation in vpbe_set_output()
>   Capture: Use kmalloc_array() in vpfe_probe()
>   Capture: Delete three error messages for a failed memory allocation
>   Capture: Improve another size determination in vpfe_probe()
>   Capture: Delete an unnecessary variable initialisation in vpfe_probe()
>   Capture: Improve another size determination in vpfe_enum_input()
>   Capture: Combine substrings for an error message in vpfe_enum_input()
>   Capture: Improve another size determination in vpfe_open()
>   Capture: Adjust 13 checks for null pointers
>   Capture: Delete an unnecessary variable initialisation in 11 functions
>   Capture: Move two assignments in vpfe_s_input()
>   Capture: Delete unnecessary braces in vpfe_isr()
>   Capture: Delete an unnecessary return statement in 
> vpfe_unregister_ccdc_device()
>   Capture: Use kcalloc() in vpif_probe()
>   Capture: Delete an error message for a failed memory allocation
>   Capture: Adjust ten checks for null pointers
>   Capture: Delete an unnecessary variable initialisation in vpif_querystd()
>   Capture: Delete an unnecessary variable initialisation in vpif_channel_isr()
>   Display: Use kcalloc() in vpif_probe()
>   Display: Delete an error message for a failed memory allocation
>   Display: Adjust 11 checks for null pointers
>   Display: Delete an unnecessary variable initialisation in vpif_channel_isr()
>   Display: Delete an unnecessary variable initialisation in 
> process_progressive_mode()
>
>  drivers/media/platform/davinci/vpbe.c | 93 
> ---
>  drivers/media/platform/davinci/vpfe_capture.c | 88 -
>  drivers/media/platform/davinci/vpif_capture.c | 28 
>  drivers/media/platform/davinci/vpif_display.c | 30 -
>  4 files changed, 109 insertions(+), 130 deletions(-)
>

Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 54/58] i2c: don't break long lines

[Lad, Prabhakar]

Hi Mauro,

Thanks for the patch.


On Tue, Oct 18, 2016 at 9:46 PM, Mauro Carvalho Chehab
<mche...@s-opensource.com> wrote:
> Due to the 80-cols restrictions, and latter due to checkpatch
> warnings, several strings were broken into multiple lines. This
> is not considered a good practice anymore, as it makes harder
> to grep for strings at the source code.
>
> As we're right now fixing other drivers due to KERN_CONT, we need
> to be able to identify what printk strings don't end with a "\n".
> It is a way easier to detect those if we don't break long lines.
>
> So, join those continuation lines.
>
> The patch was generated via the script below, and manually
> adjusted if needed.
>
> 
> use Text::Tabs;
> while (<>) {
> if ($next ne "") {
> $c=$_;
> if ($c =~ /^\s+\"(.*)/) {
> $c2=$1;
> $next =~ s/\"\n$//;
> $n = expand($next);
> $funpos = index($n, '(');
> $pos = index($c2, '",');
> if ($funpos && $pos > 0) {
> $s1 = substr $c2, 0, $pos + 2;
> $s2 = ' ' x ($funpos + 1) . substr $c2, $pos 
> + 2;
> $s2 =~ s/^\s+//;
>
> $s2 = ' ' x ($funpos + 1) . $s2 if ($s2 ne 
> "");
>
> print unexpand("$next$s1\n");
> print unexpand("$s2\n") if ($s2 ne "");
> } else {
> print "$next$c2\n";
> }
> $next="";
> next;
> } else {
> print $next;
> }
> $next="";
> } else {
> if (m/\"$/) {
>     if (!m/\\n\"$/) {
> $next=$_;
> next;
> }
> }
> }
> print $_;
> }
> 
>
> Signed-off-by: Mauro Carvalho Chehab <mche...@s-opensource.com>

Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 22/57] [media] davinci: don't break long lines

[Lad, Prabhakar]

Hi Mauro,

Thanks for the patch.

On Fri, Oct 14, 2016 at 9:20 PM, Mauro Carvalho Chehab
<mche...@s-opensource.com> wrote:
> Due to the 80-cols checkpatch warnings, several strings
> were broken into multiple lines. This is not considered
> a good practice anymore, as it makes harder to grep for
> strings at the source code. So, join those continuation
> lines.
>
> Signed-off-by: Mauro Carvalho Chehab <mche...@s-opensource.com>

Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2.2 10/10] v4l: Add 16-bit raw bayer pixel format definitions

[Lad, Prabhakar]

On Thu, Jul 7, 2016 at 7:48 AM, Sakari Ailus
<sakari.ai...@linux.intel.com> wrote:
> The formats added by this patch are:
>
> V4L2_PIX_FMT_SBGGR16
> V4L2_PIX_FMT_SGBRG16
> V4L2_PIX_FMT_SGRBG16
>
> V4L2_PIX_FMT_SRGGB16 already existed before the patch. Rework the
> documentation to match that of the other sample depths.
>
> Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>
> ---

Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Cheers,
--Prabhakar Lad

>  Documentation/DocBook/media/v4l/pixfmt-sbggr16.xml | 81 ---
>  Documentation/DocBook/media/v4l/pixfmt-srggb16.xml | 91 
> ++
>  Documentation/DocBook/media/v4l/pixfmt.xml |  2 +-
>  include/uapi/linux/videodev2.h |  3 +
>  4 files changed, 95 insertions(+), 82 deletions(-)
>  delete mode 100644 Documentation/DocBook/media/v4l/pixfmt-sbggr16.xml
>  create mode 100644 Documentation/DocBook/media/v4l/pixfmt-srggb16.xml
>
> diff --git a/Documentation/DocBook/media/v4l/pixfmt-sbggr16.xml 
> b/Documentation/DocBook/media/v4l/pixfmt-sbggr16.xml
> deleted file mode 100644
> index 789160565..000
> --- a/Documentation/DocBook/media/v4l/pixfmt-sbggr16.xml
> +++ /dev/null
> @@ -1,81 +0,0 @@
> -
> -  
> -V4L2_PIX_FMT_SBGGR16 ('BYR2')
> -
> -  
> -  
> -V4L2_PIX_FMT_SBGGR16
> -Bayer RGB format
> -  
> -  
> -Description
> -
> -This format is similar to  -linkend="V4L2-PIX-FMT-SBGGR8">
> -V4L2_PIX_FMT_SBGGR8, except each pixel has
> -a depth of 16 bits. The least significant byte is stored at lower
> -memory addresses (little-endian).
> -
> -
> -  V4L2_PIX_FMT_SBGGR16 4  4
> -pixel image
> -
> -  
> -   Byte Order.
> -   Each cell is one byte.
> - 
> -   
> - 
> - 
> -   
> - start+0:
> - B00low
> - B00high
> - G01low
> - G01high
> - B02low
> - B02high
> - G03low
> - G03high
> -   
> -   
> - start+8:
> - G10low
> - G10high
> - R11low
> - R11high
> - G12low
> - G12high
> - R13low
> - R13high
> -   
> -   
> - start+16:
> - B20low
> - B20high
> - G21low
> - G21high
> - B22low
> - B22high
> - G23low
> - G23high
> -   
> -   
> - start+24:
> - G30low
> - G30high
> - R31low
> - R31high
> - G32low
> - G32high
> - R33low
> - R33high
> -   
> - 
> -   
> - 
> -   
> -  
> -
> -  
> -
> diff --git a/Documentation/DocBook/media/v4l/pixfmt-srggb16.xml 
> b/Documentation/DocBook/media/v4l/pixfmt-srggb16.xml
> new file mode 100644
> index 000..590266f
> --- /dev/null
> +++ b/Documentation/DocBook/media/v4l/pixfmt-srggb16.xml
> @@ -0,0 +1,91 @@
> +
> +  
> +   V4L2_PIX_FMT_SRGGB16 ('RG16'),
> +V4L2_PIX_FMT_SGRBG16 ('GR16'),
> +V4L2_PIX_FMT_SGBRG16 ('GB16'),
> +V4L2_PIX_FMT_SBGGR16 ('BYR2')
> +
> +   
> +  
> +  
> +id="V4L2-PIX-FMT-SRGGB16">V4L2_PIX_FMT_SRGGB16
> +id="V4L2-PIX-FMT-SGRBG16">V4L2_PIX_FMT_SGRBG16
> +id="V4L2-PIX-FMT-SGBRG16">V4L2_PIX_FMT_SGBRG16
> +id="V4L2-PIX-FMT-SBGGR16">V4L2_PIX_FMT_SBGGR16
> +   16-bit Bayer formats
> +  
> +  
> +   Description
> +
> +   These four pixel formats are raw sRGB / Bayer formats with
> +16 bits per colour. Each colour component is stored in a 16-bit word.
> +Each n-pixel row contains n/2 green samples and n/2 blue or red
> +samples, with alternating red and blue rows. Bytes are stored in
> +memory in little endian order. They are conventionally described
> +as GRGR... BGBG..., RGRG... GBGB..., etc. Below is an example of one of these
> +formats:
> +
> +
> +  V4L2_PIX_FMT_SBGGR16 4  4
> +pixel image
> +
> +  
> +   Byte Order.
> +   Each cell is one byte.
> + 
> +   

Re: [PATCH 7/9] vpbe_display: convert g/s_crop to g/s_selection.

[Lad, Prabhakar]

On Mon, Jul 4, 2016 at 9:32 AM, Hans Verkuil <hverk...@xs4all.nl> wrote:
> From: Hans Verkuil <hans.verk...@cisco.com>
>
> This is part of a final push to convert all drivers to g/s_selection.
>
> Signed-off-by: Hans Verkuil <hans.verk...@cisco.com>
> Cc: Prabhakar Lad <prabhakar.cse...@gmail.com>

Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Cheers,
--Prabhakar Lad

> ---
>  drivers/media/platform/davinci/vpbe_display.c | 65 
> +++
>  1 file changed, 37 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/media/platform/davinci/vpbe_display.c 
> b/drivers/media/platform/davinci/vpbe_display.c
> index 0abcdfe..b4a8cd2 100644
> --- a/drivers/media/platform/davinci/vpbe_display.c
> +++ b/drivers/media/platform/davinci/vpbe_display.c
> @@ -441,7 +441,7 @@ vpbe_disp_calculate_scale_factor(struct vpbe_display 
> *disp_dev,
> /*
>  * Application initially set the image format. Current display
>  * size is obtained from the vpbe display controller. expected_xsize
> -* and expected_ysize are set through S_CROP ioctl. Based on this,
> +* and expected_ysize are set through S_SELECTION ioctl. Based on 
> this,
>  * driver will calculate the scale factors for vertical and
>  * horizontal direction so that the image is displayed scaled
>  * and expanded. Application uses expansion to display the image
> @@ -650,24 +650,23 @@ static int vpbe_display_querycap(struct file *file, 
> void  *priv,
> return 0;
>  }
>
> -static int vpbe_display_s_crop(struct file *file, void *priv,
> -const struct v4l2_crop *crop)
> +static int vpbe_display_s_selection(struct file *file, void *priv,
> +struct v4l2_selection *sel)
>  {
> struct vpbe_layer *layer = video_drvdata(file);
> struct vpbe_display *disp_dev = layer->disp_dev;
> struct vpbe_device *vpbe_dev = disp_dev->vpbe_dev;
> struct osd_layer_config *cfg = >layer_info.config;
> struct osd_state *osd_device = disp_dev->osd_device;
> -   struct v4l2_rect rect = crop->c;
> +   struct v4l2_rect rect = sel->r;
> int ret;
>
> v4l2_dbg(1, debug, _dev->v4l2_dev,
> -   "VIDIOC_S_CROP, layer id = %d\n", layer->device_id);
> +   "VIDIOC_S_SELECTION, layer id = %d\n", layer->device_id);
>
> -   if (crop->type != V4L2_BUF_TYPE_VIDEO_OUTPUT) {
> -   v4l2_err(_dev->v4l2_dev, "Invalid buf type\n");
> +   if (sel->type != V4L2_BUF_TYPE_VIDEO_OUTPUT ||
> +   sel->target != V4L2_SEL_TGT_CROP)
> return -EINVAL;
> -   }
>
> if (rect.top < 0)
> rect.top = 0;
> @@ -715,32 +714,45 @@ static int vpbe_display_s_crop(struct file *file, void 
> *priv,
> else
> osd_device->ops.set_interpolation_filter(osd_device, 0);
>
> +   sel->r = rect;
> return 0;
>  }
>
> -static int vpbe_display_g_crop(struct file *file, void *priv,
> -struct v4l2_crop *crop)
> +static int vpbe_display_g_selection(struct file *file, void *priv,
> +   struct v4l2_selection *sel)
>  {
> struct vpbe_layer *layer = video_drvdata(file);
> struct osd_layer_config *cfg = >layer_info.config;
> struct vpbe_device *vpbe_dev = layer->disp_dev->vpbe_dev;
> struct osd_state *osd_device = layer->disp_dev->osd_device;
> -   struct v4l2_rect *rect = >c;
> +   struct v4l2_rect *rect = >r;
>
> v4l2_dbg(1, debug, _dev->v4l2_dev,
> -   "VIDIOC_G_CROP, layer id = %d\n",
> +   "VIDIOC_G_SELECTION, layer id = %d\n",
> layer->device_id);
>
> -   if (crop->type != V4L2_BUF_TYPE_VIDEO_OUTPUT) {
> -   v4l2_err(_dev->v4l2_dev, "Invalid buf type\n");
> +   if (sel->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
> +   return -EINVAL;
> +
> +   switch (sel->target) {
> +   case V4L2_SEL_TGT_CROP:
> +   osd_device->ops.get_layer_config(osd_device,
> +layer->layer_info.id, cfg);
> +   rect->top = cfg->ypos;
> +   rect->left = cfg->xpos;
> +   rect->width = cfg->xsize;
> +   rect->height = cfg->ysize;
> +   break;
> +   case V4L2_SEL_TGT_CROP_DEFAULT:
> +   case V4L2_SEL_TGT_CROP_BO

Re: [PATCH 03/14] davinci: drop unused control callbacks

[Lad, Prabhakar]

On Mon, Jul 4, 2016 at 9:34 AM, Hans Verkuil <hverk...@xs4all.nl> wrote:
> From: Hans Verkuil <hans.verk...@cisco.com>
>
> These callbacks are no longer used since the davinci drivers use the
> control framework.
>
> Signed-off-by: Hans Verkuil <hans.verk...@cisco.com>
> Cc: Prabhakar Lad <prabhakar.cse...@gmail.com>

Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Cheers,
--Prabhakar Lad

> ---
>  drivers/media/platform/davinci/ccdc_hw_device.h | 7 ---
>  1 file changed, 7 deletions(-)
>
> diff --git a/drivers/media/platform/davinci/ccdc_hw_device.h 
> b/drivers/media/platform/davinci/ccdc_hw_device.h
> index 86b9b35..ae5605d 100644
> --- a/drivers/media/platform/davinci/ccdc_hw_device.h
> +++ b/drivers/media/platform/davinci/ccdc_hw_device.h
> @@ -80,13 +80,6 @@ struct ccdc_hw_ops {
> /* Pointer to function to get line length */
> unsigned int (*get_line_length) (void);
>
> -   /* Query CCDC control IDs */
> -   int (*queryctrl)(struct v4l2_queryctrl *qctrl);
> -   /* Set CCDC control */
> -   int (*set_control)(struct v4l2_control *ctrl);
> -   /* Get CCDC control */
> -   int (*get_control)(struct v4l2_control *ctrl);
> -
> /* Pointer to function to set frame buffer address */
> void (*setfbaddr) (unsigned long addr);
> /* Pointer to function to get field id */
> --
> 2.8.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2.2 09/10] v4l: 16-bit BGGR is always 16 bits

[Lad, Prabhakar]

On Thu, Jul 7, 2016 at 7:48 AM, Sakari Ailus
<sakari.ai...@linux.intel.com> wrote:
> The V4L2_PIX_FMT_SBGGR16 format is documented to contain samples of fewer
> than 16 bits. However, we do have specific definitions for smaller sample
> sizes. Therefore, this note is redundant from the API point of view.
>
> Currently only two drivers, am437x and davinci, use the V4L2_PIX_FMT_SBGGR16
> pixelformat currently. The sampling precision is understood to be 16 bits in
> all current cases.
>
> Remove the note on sampling precision.
>
> Signed-off-by: Sakari Ailus <sakari.ai...@linux.intel.com>
> ---

Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Cheers,
--Prabhakar Lad

>  Documentation/DocBook/media/v4l/pixfmt-sbggr16.xml | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/Documentation/DocBook/media/v4l/pixfmt-sbggr16.xml 
> b/Documentation/DocBook/media/v4l/pixfmt-sbggr16.xml
> index 6494b05..789160565 100644
> --- a/Documentation/DocBook/media/v4l/pixfmt-sbggr16.xml
> +++ b/Documentation/DocBook/media/v4l/pixfmt-sbggr16.xml
> @@ -14,9 +14,7 @@
>  linkend="V4L2-PIX-FMT-SBGGR8">
>  V4L2_PIX_FMT_SBGGR8, except each pixel has
>  a depth of 16 bits. The least significant byte is stored at lower
> -memory addresses (little-endian). Note the actual sampling precision
> -may be lower than 16 bits, for example 10 bits per pixel with values
> -in range 0 to 1023.
> +memory addresses (little-endian).
>
>  
>V4L2_PIX_FMT_SBGGR16 4  4
> --
> 2.7.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/9] vpfe_capture: convert g/s_crop to g/s_selection.

[Lad, Prabhakar]

On Mon, Jul 4, 2016 at 9:32 AM, Hans Verkuil <hverk...@xs4all.nl> wrote:
> From: Hans Verkuil <hans.verk...@cisco.com>
>
> This is part of a final push to convert all drivers to g/s_selection.
>
> Signed-off-by: Hans Verkuil <hans.verk...@cisco.com>
> Cc: Prabhakar Lad <prabhakar.cse...@gmail.com>

Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Cheers,
--Prabhakar Lad

> ---
>  drivers/media/platform/davinci/vpfe_capture.c | 52 
> +--
>  1 file changed, 34 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/media/platform/davinci/vpfe_capture.c 
> b/drivers/media/platform/davinci/vpfe_capture.c
> index 7767e07..6efb2f1 100644
> --- a/drivers/media/platform/davinci/vpfe_capture.c
> +++ b/drivers/media/platform/davinci/vpfe_capture.c
> @@ -1610,38 +1610,53 @@ static int vpfe_cropcap(struct file *file, void *priv,
>
> v4l2_dbg(1, debug, _dev->v4l2_dev, "vpfe_cropcap\n");
>
> -   if (vpfe_dev->std_index >= ARRAY_SIZE(vpfe_standards))
> +   if (crop->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> return -EINVAL;
> +   /* If std_index is invalid, then just return (== 1:1 aspect) */
> +   if (vpfe_dev->std_index >= ARRAY_SIZE(vpfe_standards))
> +   return 0;
>
> -   memset(crop, 0, sizeof(struct v4l2_cropcap));
> -   crop->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> -   crop->bounds.width = crop->defrect.width =
> -   vpfe_standards[vpfe_dev->std_index].width;
> -   crop->bounds.height = crop->defrect.height =
> -   vpfe_standards[vpfe_dev->std_index].height;
> crop->pixelaspect = vpfe_standards[vpfe_dev->std_index].pixelaspect;
> return 0;
>  }
>
> -static int vpfe_g_crop(struct file *file, void *priv,
> -struct v4l2_crop *crop)
> +static int vpfe_g_selection(struct file *file, void *priv,
> +   struct v4l2_selection *sel)
>  {
> struct vpfe_device *vpfe_dev = video_drvdata(file);
>
> -   v4l2_dbg(1, debug, _dev->v4l2_dev, "vpfe_g_crop\n");
> +   v4l2_dbg(1, debug, _dev->v4l2_dev, "vpfe_g_selection\n");
>
> -   crop->c = vpfe_dev->crop;
> +   if (sel->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
> +   return -EINVAL;
> +
> +   switch (sel->target) {
> +   case V4L2_SEL_TGT_CROP:
> +   sel->r = vpfe_dev->crop;
> +   break;
> +   case V4L2_SEL_TGT_CROP_DEFAULT:
> +   case V4L2_SEL_TGT_CROP_BOUNDS:
> +   sel->r.width = vpfe_standards[vpfe_dev->std_index].width;
> +   sel->r.height = vpfe_standards[vpfe_dev->std_index].height;
> +   break;
> +   default:
> +   return -EINVAL;
> +   }
> return 0;
>  }
>
> -static int vpfe_s_crop(struct file *file, void *priv,
> -const struct v4l2_crop *crop)
> +static int vpfe_s_selection(struct file *file, void *priv,
> +   struct v4l2_selection *sel)
>  {
> struct vpfe_device *vpfe_dev = video_drvdata(file);
> -   struct v4l2_rect rect = crop->c;
> +   struct v4l2_rect rect = sel->r;
> int ret = 0;
>
> -   v4l2_dbg(1, debug, _dev->v4l2_dev, "vpfe_s_crop\n");
> +   v4l2_dbg(1, debug, _dev->v4l2_dev, "vpfe_s_selection\n");
> +
> +   if (sel->type != V4L2_BUF_TYPE_VIDEO_CAPTURE ||
> +   sel->target != V4L2_SEL_TGT_CROP)
> +   return -EINVAL;
>
> if (vpfe_dev->started) {
> /* make sure streaming is not started */
> @@ -1669,7 +1684,7 @@ static int vpfe_s_crop(struct file *file, void *priv,
> vpfe_dev->std_info.active_pixels) ||
> (rect.top + rect.height >
> vpfe_dev->std_info.active_lines)) {
> -   v4l2_err(_dev->v4l2_dev, "Error in S_CROP params\n");
> +   v4l2_err(_dev->v4l2_dev, "Error in S_SELECTION 
> params\n");
> ret = -EINVAL;
> goto unlock_out;
> }
> @@ -1682,6 +1697,7 @@ static int vpfe_s_crop(struct file *file, void *priv,
> vpfe_dev->fmt.fmt.pix.bytesperline *
> vpfe_dev->fmt.fmt.pix.height;
> vpfe_dev->crop = rect;
> +   sel->r = rect;
>  unlock_out:
> mutex_unlock(_dev->lock);
> return ret;
> @@ -1760,8 +1776,8 @@ static const struct v4l2_ioctl_ops vpfe_ioctl_ops = {
> .vidioc_streamon = vpfe_streamon,
>

Re: [PATCH 2/2] [media] tvp5150: propagate I2C write error in .s_register callback

[Lad, Prabhakar]

On Fri, Apr 15, 2016 at 2:00 AM, Javier Martinez Canillas
<jav...@osg.samsung.com> wrote:
> The tvp5150_write() function can fail so don't return 0 unconditionally
> in tvp5150_s_register() but propagate what's returned by tvp5150_write().
>
> Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
>
Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Cheers,
--Prabhakar Lad

> ---
>
>  drivers/media/i2c/tvp5150.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> index 4a2e851b6a3b..7be456d1b071 100644
> --- a/drivers/media/i2c/tvp5150.c
> +++ b/drivers/media/i2c/tvp5150.c
> @@ -1161,8 +1161,7 @@ static int tvp5150_g_register(struct v4l2_subdev *sd, 
> struct v4l2_dbg_register *
>
>  static int tvp5150_s_register(struct v4l2_subdev *sd, const struct 
> v4l2_dbg_register *reg)
>  {
> -   tvp5150_write(sd, reg->reg & 0xff, reg->val & 0xff);
> -   return 0;
> +   return tvp5150_write(sd, reg->reg & 0xff, reg->val & 0xff);
>  }
>  #endif
>
> --
> 2.5.5
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] [media] tvp5150: return I2C write operation failure to callers

[Lad, Prabhakar]

On Fri, Apr 15, 2016 at 2:00 AM, Javier Martinez Canillas
<jav...@osg.samsung.com> wrote:
> The tvp5150_write() function calls i2c_smbus_write_byte_data() that
> can fail but does not propagate the error to the caller. Instead it
> just prints a debug, so callers can't know if the operation failed.
>
> So change the function to return the error code to the caller so it
> knows that the write failed and also print an error instead of just
> printing a debug information.
>
> While being there remove the inline keyword from tvp5150_write() to
> make it consistent with tvp5150_read() and also because it's called
> in a lot of places, so making inline is in fact counter productive
> since it makes the kernel image size to be much bigger (~16 KiB).
>
> Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>

Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Cheers,
--Prabhakar Lad

> ---
>
>  drivers/media/i2c/tvp5150.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/i2c/tvp5150.c b/drivers/media/i2c/tvp5150.c
> index e5003d94f262..4a2e851b6a3b 100644
> --- a/drivers/media/i2c/tvp5150.c
> +++ b/drivers/media/i2c/tvp5150.c
> @@ -83,7 +83,7 @@ static int tvp5150_read(struct v4l2_subdev *sd, unsigned 
> char addr)
> return rc;
>  }
>
> -static inline void tvp5150_write(struct v4l2_subdev *sd, unsigned char addr,
> +static int tvp5150_write(struct v4l2_subdev *sd, unsigned char addr,
>  unsigned char value)
>  {
> struct i2c_client *c = v4l2_get_subdevdata(sd);
> @@ -92,7 +92,9 @@ static inline void tvp5150_write(struct v4l2_subdev *sd, 
> unsigned char addr,
> v4l2_dbg(2, debug, sd, "tvp5150: writing 0x%02x 0x%02x\n", addr, 
> value);
> rc = i2c_smbus_write_byte_data(c, addr, value);
> if (rc < 0)
> -   v4l2_dbg(0, debug, sd, "i2c i/o error: rc == %d\n", rc);
> +   v4l2_err(sd, "i2c i/o error: rc == %d\n", rc);
> +
> +   return rc;
>  }
>
>  static void dump_reg_range(struct v4l2_subdev *sd, char *s, u8 init,
> --
> 2.5.5
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv4 06/13] staging/media: convert drivers to use the new vb2_queue dev field

[Lad, Prabhakar]

Hi Hans,

On Sat, Apr 23, 2016 at 12:03 PM, Hans Verkuil <hverk...@xs4all.nl> wrote:
> From: Hans Verkuil <hans.verk...@cisco.com>
>
> Stop using alloc_ctx and just fill in the device pointer.
>
> Signed-off-by: Hans Verkuil <hans.verk...@cisco.com>
> Cc: "Lad, Prabhakar" <prabhakar.cse...@gmail.com>
> Reviewed-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>

Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Cheers,
--Prabhakar Lad

> ---
>  drivers/staging/media/davinci_vpfe/vpfe_video.c | 10 +-
>  drivers/staging/media/davinci_vpfe/vpfe_video.h |  2 --
>  drivers/staging/media/omap4iss/iss_video.c  | 10 +-
>  drivers/staging/media/omap4iss/iss_video.h  |  1 -
>  4 files changed, 2 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c 
> b/drivers/staging/media/davinci_vpfe/vpfe_video.c
> index ea3ddec..77e66e7 100644
> --- a/drivers/staging/media/davinci_vpfe/vpfe_video.c
> +++ b/drivers/staging/media/davinci_vpfe/vpfe_video.c
> @@ -542,7 +542,6 @@ static int vpfe_release(struct file *file)
> video->io_usrs = 0;
> /* Free buffers allocated */
> vb2_queue_release(>buffer_queue);
> -   vb2_dma_contig_cleanup_ctx(video->alloc_ctx);
> }
> /* Decrement device users counter */
> video->usrs--;
> @@ -1115,7 +1114,6 @@ vpfe_buffer_queue_setup(struct vb2_queue *vq,
>
> *nplanes = 1;
> sizes[0] = size;
> -   alloc_ctxs[0] = video->alloc_ctx;
> v4l2_dbg(1, debug, _dev->v4l2_dev,
>  "nbuffers=%d, size=%lu\n", *nbuffers, size);
> return 0;
> @@ -1350,12 +1348,6 @@ static int vpfe_reqbufs(struct file *file, void *priv,
> video->memory = req_buf->memory;
>
> /* Initialize videobuf2 queue as per the buffer type */
> -   video->alloc_ctx = vb2_dma_contig_init_ctx(vpfe_dev->pdev);
> -   if (IS_ERR(video->alloc_ctx)) {
> -   v4l2_err(_dev->v4l2_dev, "Failed to get the context\n");
> -   return PTR_ERR(video->alloc_ctx);
> -   }
> -
> q = >buffer_queue;
> q->type = req_buf->type;
> q->io_modes = VB2_MMAP | VB2_USERPTR;
> @@ -1365,11 +1357,11 @@ static int vpfe_reqbufs(struct file *file, void *priv,
> q->mem_ops = _dma_contig_memops;
> q->buf_struct_size = sizeof(struct vpfe_cap_buffer);
> q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> +   q->dev = vpfe_dev->pdev;
>
> ret = vb2_queue_init(q);
> if (ret) {
> v4l2_err(_dev->v4l2_dev, "vb2_queue_init() failed\n");
> -   vb2_dma_contig_cleanup_ctx(vpfe_dev->pdev);
> return ret;
> }
>
> diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.h 
> b/drivers/staging/media/davinci_vpfe/vpfe_video.h
> index 653334d..aaec440 100644
> --- a/drivers/staging/media/davinci_vpfe/vpfe_video.h
> +++ b/drivers/staging/media/davinci_vpfe/vpfe_video.h
> @@ -123,8 +123,6 @@ struct vpfe_video_device {
> /* Used to store pixel format */
> struct v4l2_format  fmt;
> struct vb2_queuebuffer_queue;
> -   /* allocator-specific contexts for each plane */
> -   struct vb2_alloc_ctx *alloc_ctx;
> /* Queue of filled frames */
> struct list_headdma_queue;
> spinlock_t  irqlock;
> diff --git a/drivers/staging/media/omap4iss/iss_video.c 
> b/drivers/staging/media/omap4iss/iss_video.c
> index cf8da23..3c077e3 100644
> --- a/drivers/staging/media/omap4iss/iss_video.c
> +++ b/drivers/staging/media/omap4iss/iss_video.c
> @@ -310,8 +310,6 @@ static int iss_video_queue_setup(struct vb2_queue *vq,
> if (sizes[0] == 0)
> return -EINVAL;
>
> -   alloc_ctxs[0] = video->alloc_ctx;
> -
> *count = min(*count, video->capture_mem / PAGE_ALIGN(sizes[0]));
>
> return 0;
> @@ -1017,13 +1015,6 @@ static int iss_video_open(struct file *file)
> goto done;
> }
>
> -   video->alloc_ctx = vb2_dma_contig_init_ctx(video->iss->dev);
> -   if (IS_ERR(video->alloc_ctx)) {
> -   ret = PTR_ERR(video->alloc_ctx);
> -   omap4iss_put(video->iss);
> -   goto done;
> -   }
> -
> q = >queue;
>
> q->type = video->type;
> @@ -1033,6 +1024,7 @@ static int iss_video_open(struct file *f

Re: [PATCHv3 05/12] staging/media: convert drivers to use the new vb2_queue dev field

[Lad, Prabhakar]

Hi Hans,

Thanks for the patch.

On Fri, Apr 22, 2016 at 9:38 AM, Hans Verkuil <hverk...@xs4all.nl> wrote:
> From: Hans Verkuil <hans.verk...@cisco.com>
>
> Stop using alloc_ctx and just fill in the device pointer.
>
> Signed-off-by: Hans Verkuil <hans.verk...@cisco.com>
> Cc: "Lad, Prabhakar" <prabhakar.cse...@gmail.com>
> Cc: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> ---
>  drivers/staging/media/davinci_vpfe/vpfe_video.c | 10 +-
>  drivers/staging/media/davinci_vpfe/vpfe_video.h |  2 --

For the above

Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Cheers,
--Prabhakar Lad

>  drivers/staging/media/omap4iss/iss_video.c  | 10 +-
>  drivers/staging/media/omap4iss/iss_video.h  |  1 -
>  4 files changed, 2 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c 
> b/drivers/staging/media/davinci_vpfe/vpfe_video.c
> index ea3ddec..77e66e7 100644
> --- a/drivers/staging/media/davinci_vpfe/vpfe_video.c
> +++ b/drivers/staging/media/davinci_vpfe/vpfe_video.c
> @@ -542,7 +542,6 @@ static int vpfe_release(struct file *file)
> video->io_usrs = 0;
> /* Free buffers allocated */
> vb2_queue_release(>buffer_queue);
> -   vb2_dma_contig_cleanup_ctx(video->alloc_ctx);
> }
> /* Decrement device users counter */
> video->usrs--;
> @@ -1115,7 +1114,6 @@ vpfe_buffer_queue_setup(struct vb2_queue *vq,
>
> *nplanes = 1;
> sizes[0] = size;
> -   alloc_ctxs[0] = video->alloc_ctx;
> v4l2_dbg(1, debug, _dev->v4l2_dev,
>  "nbuffers=%d, size=%lu\n", *nbuffers, size);
> return 0;
> @@ -1350,12 +1348,6 @@ static int vpfe_reqbufs(struct file *file, void *priv,
> video->memory = req_buf->memory;
>
> /* Initialize videobuf2 queue as per the buffer type */
> -   video->alloc_ctx = vb2_dma_contig_init_ctx(vpfe_dev->pdev);
> -   if (IS_ERR(video->alloc_ctx)) {
> -   v4l2_err(_dev->v4l2_dev, "Failed to get the context\n");
> -   return PTR_ERR(video->alloc_ctx);
> -   }
> -
> q = >buffer_queue;
> q->type = req_buf->type;
> q->io_modes = VB2_MMAP | VB2_USERPTR;
> @@ -1365,11 +1357,11 @@ static int vpfe_reqbufs(struct file *file, void *priv,
> q->mem_ops = _dma_contig_memops;
> q->buf_struct_size = sizeof(struct vpfe_cap_buffer);
> q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> +   q->dev = vpfe_dev->pdev;
>
> ret = vb2_queue_init(q);
> if (ret) {
> v4l2_err(_dev->v4l2_dev, "vb2_queue_init() failed\n");
> -   vb2_dma_contig_cleanup_ctx(vpfe_dev->pdev);
> return ret;
> }
>
> diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.h 
> b/drivers/staging/media/davinci_vpfe/vpfe_video.h
> index 653334d..aaec440 100644
> --- a/drivers/staging/media/davinci_vpfe/vpfe_video.h
> +++ b/drivers/staging/media/davinci_vpfe/vpfe_video.h
> @@ -123,8 +123,6 @@ struct vpfe_video_device {
> /* Used to store pixel format */
> struct v4l2_format  fmt;
> struct vb2_queuebuffer_queue;
> -   /* allocator-specific contexts for each plane */
> -   struct vb2_alloc_ctx *alloc_ctx;
> /* Queue of filled frames */
> struct list_headdma_queue;
> spinlock_t  irqlock;
> diff --git a/drivers/staging/media/omap4iss/iss_video.c 
> b/drivers/staging/media/omap4iss/iss_video.c
> index cf8da23..3c077e3 100644
> --- a/drivers/staging/media/omap4iss/iss_video.c
> +++ b/drivers/staging/media/omap4iss/iss_video.c
> @@ -310,8 +310,6 @@ static int iss_video_queue_setup(struct vb2_queue *vq,
> if (sizes[0] == 0)
> return -EINVAL;
>
> -   alloc_ctxs[0] = video->alloc_ctx;
> -
> *count = min(*count, video->capture_mem / PAGE_ALIGN(sizes[0]));
>
> return 0;
> @@ -1017,13 +1015,6 @@ static int iss_video_open(struct file *file)
> goto done;
> }
>
> -   video->alloc_ctx = vb2_dma_contig_init_ctx(video->iss->dev);
> -   if (IS_ERR(video->alloc_ctx)) {
> -   ret = PTR_ERR(video->alloc_ctx);
> -   omap4iss_put(video->iss);
> -   goto done;
> -   }
> -
> q = >queue;
>
> q->type = video->type;
> @@ -1033,6 +1024,7 @@ static int iss_vi

Re: [PATCHv4 07/13] media/platform: convert drivers to use the new vb2_queue dev field

[Lad, Prabhakar]

Hi Hans,

Thanks for the patch.

On Sat, Apr 23, 2016 at 12:03 PM, Hans Verkuil <hverk...@xs4all.nl> wrote:
> From: Hans Verkuil <hans.verk...@cisco.com>
>
> Stop using alloc_ctx and just fill in the device pointer.
>
> Signed-off-by: Hans Verkuil <hans.verk...@cisco.com>
> Cc: "Lad, Prabhakar" <prabhakar.cse...@gmail.com>
> Cc: Scott Jiang <scott.jiang.li...@gmail.com>
> Acked-by: Philipp Zabel <p.za...@pengutronix.de>
> ---
>  drivers/media/platform/am437x/am437x-vpfe.c| 10 +-
>  drivers/media/platform/am437x/am437x-vpfe.h|  2 --
>  drivers/media/platform/davinci/vpbe_display.c  | 12 +---
>  drivers/media/platform/davinci/vpif_capture.c  | 11 +--
>  drivers/media/platform/davinci/vpif_capture.h  |  2 --
>  drivers/media/platform/davinci/vpif_display.c  | 11 +--
>  drivers/media/platform/davinci/vpif_display.h  |  2 --
>  include/media/davinci/vpbe_display.h   |  2 --
>  11 files changed, 8 insertions(+), 76 deletions(-)
>

For all the above

Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Cheers,
--Prabhakar Lad

> diff --git a/drivers/media/platform/am437x/am437x-vpfe.c 
> b/drivers/media/platform/am437x/am437x-vpfe.c
> index e749eb7..d22b09d 100644
> --- a/drivers/media/platform/am437x/am437x-vpfe.c
> +++ b/drivers/media/platform/am437x/am437x-vpfe.c
> @@ -1915,7 +1915,6 @@ static int vpfe_queue_setup(struct vb2_queue *vq,
>
> if (vq->num_buffers + *nbuffers < 3)
> *nbuffers = 3 - vq->num_buffers;
> -   alloc_ctxs[0] = vpfe->alloc_ctx;
>
> if (*nplanes) {
> if (sizes[0] < size)
> @@ -2364,13 +2363,6 @@ static int vpfe_probe_complete(struct vpfe_device 
> *vpfe)
> goto probe_out;
>
> /* Initialize videobuf2 queue as per the buffer type */
> -   vpfe->alloc_ctx = vb2_dma_contig_init_ctx(vpfe->pdev);
> -   if (IS_ERR(vpfe->alloc_ctx)) {
> -   vpfe_err(vpfe, "Failed to get the context\n");
> -   err = PTR_ERR(vpfe->alloc_ctx);
> -   goto probe_out;
> -   }
> -
> q = >buffer_queue;
> q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> q->io_modes = VB2_MMAP | VB2_DMABUF | VB2_READ;
> @@ -2381,11 +2373,11 @@ static int vpfe_probe_complete(struct vpfe_device 
> *vpfe)
> q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> q->lock = >lock;
> q->min_buffers_needed = 1;
> +   q->dev = vpfe->pdev;
>
> err = vb2_queue_init(q);
> if (err) {
> vpfe_err(vpfe, "vb2_queue_init() failed\n");
> -   vb2_dma_contig_cleanup_ctx(vpfe->alloc_ctx);
> goto probe_out;
> }
>
> diff --git a/drivers/media/platform/am437x/am437x-vpfe.h 
> b/drivers/media/platform/am437x/am437x-vpfe.h
> index 777bf97..17d7aa4 100644
> --- a/drivers/media/platform/am437x/am437x-vpfe.h
> +++ b/drivers/media/platform/am437x/am437x-vpfe.h
> @@ -264,8 +264,6 @@ struct vpfe_device {
> struct v4l2_rect crop;
> /* Buffer queue used in video-buf */
> struct vb2_queue buffer_queue;
> -   /* Allocator-specific contexts for each plane */
> -   struct vb2_alloc_ctx *alloc_ctx;
> /* Queue of filled frames */
> struct list_head dma_queue;
> /* IRQ lock for DMA queue */
> diff --git a/drivers/media/platform/blackfin/bfin_capture.c 
> b/drivers/media/platform/blackfin/bfin_capture.c
> index d0092da..1e244287 100644
> --- a/drivers/media/platform/blackfin/bfin_capture.c
> +++ b/drivers/media/platform/blackfin/bfin_capture.c
> @@ -91,8 +91,6 @@ struct bcap_device {
> struct bcap_buffer *cur_frm;
> /* buffer queue used in videobuf2 */
> struct vb2_queue buffer_queue;
> -   /* allocator-specific contexts for each plane */
> -   struct vb2_alloc_ctx *alloc_ctx;
> /* queue of filled frames */
> struct list_head dma_queue;
> /* used in videobuf2 callback */
> @@ -209,7 +207,6 @@ static int bcap_queue_setup(struct vb2_queue *vq,
>
> if (vq->num_buffers + *nbuffers < 2)
> *nbuffers = 2;
> -   alloc_ctxs[0] = bcap_dev->alloc_ctx;
>
> if (*nplanes)
> return sizes[0] < bcap_dev->fmt.sizeimage ? -EINVAL : 0;
> @@ -820,12 +817,6 @@ static int bcap_probe(struct platform_device *pdev)
> }
> bcap_dev->ppi->priv = bcap_dev;
>
> -   bcap_dev->alloc_ctx = vb2_dma_contig_init_ctx(>dev);
> -   if (IS_ERR(bcap_dev->alloc_ctx)) {
> -

Re: [PATCH v6 0/2] media: Add entity types

[Lad, Prabhakar]

Hi Laurent,

Thanks for the patches.


On Thu, Mar 24, 2016 at 8:50 AM, Laurent Pinchart
<laurent.pinchart+rene...@ideasonboard.com> wrote:
> Hello,
>
> This patch series adds an obj_type field to the media entity structure. It
> is a resend of v5 with the MEDIA_ENTITY_TYPE_INVALID type replaced by
> MEDIA_ENTITY_TYPE_BASE to identify media entity instances not embedded in
> another structure.
>
> Cc: Kyungmin Park <kyungmin.p...@samsung.com>
> Cc: Sylwester Nawrocki <s.nawro...@samsung.com>
> Cc: Prabhakar Lad <prabhakar.cse...@gmail.com>
>
> Laurent Pinchart (2):
>   media: Add obj_type field to struct media_entity
>   media: Rename is_media_entity_v4l2_io to
> is_media_entity_v4l2_video_device
>

Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] [media] media: am437x-vpfe: ensure ret is initialized

[Lad, Prabhakar]

Hi Colin,

On Mon, Mar 21, 2016 at 11:32 PM, Colin King  wrote:
> From: Colin Ian King 
>
> ret should be initialized to 0; for example if pfe->fmt.fmt.pix.field
> is V4L2_FIELD_NONE then ret will contain garbage from the
> uninitialized state causing garbage to be returned if it is non-zero.
>
Thanks for the patch, patch [1] fixing this issue is already posted in ML.

[1] http://www.mail-archive.com/linux-media@vger.kernel.org/msg95562.html

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] [media] am437x-vfpe: fix typo in vpfe_get_app_input_index

[Lad, Prabhakar]

Hi Arnd,

Thanks for the patch.

On Mon, Mar 14, 2016 at 10:40 PM, Arnd Bergmann <a...@arndb.de> wrote:
> gcc-6 points out an obviously silly comparison in vpfe_get_app_input_index():
>
> drivers/media/platform/am437x/am437x-vpfe.c: In function 
> 'vpfe_get_app_input_index':
> drivers/media/platform/am437x/am437x-vpfe.c:1709:27: warning: self-comparison 
> always evaluats to true [-Wtautological-compare]
>client->adapter->nr == client->adapter->nr) {
>^~
>
> This was introduced in a slighly incorrect conversion, and it's
> clear that the comparison was meant to compare the iterator
> to the current subdev instead, as we do in the line above.
>
> Signed-off-by: Arnd Bergmann <a...@arndb.de>
> Fixes: d37232390fd4 ("[media] media: am437x-vpfe: match the OF node/i2c addr 
> instead of name")
> ---

Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] [media] am437x-vpfe: fix an uninitialized variable bug

[Lad, Prabhakar]

Hi Dan,

Thanks for the patch.

On Tue, Mar 15, 2016 at 7:04 AM, Dan Carpenter <dan.carpen...@oracle.com> wrote:
> If we are doing V4L2_FIELD_NONE then "ret" is used uninitialized.
>
> Fixes: 417d2e507edc ('[media] media: platform: add VPFE capture driver 
> support for AM437X')
> Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
>
Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/8] media: Rename is_media_entity_v4l2_io to is_media_entity_v4l2_video_device

[Lad, Prabhakar]

Hi Laurent,

Thanks for the patch.

On Tue, Mar 1, 2016 at 2:57 PM, Laurent Pinchart
<laurent.pinchart+rene...@ideasonboard.com> wrote:
> All users of is_media_entity_v4l2_io() (the exynos4-is, omap3isp,
> davince_vpfe and omap4iss drivers) use the function to check whether
> entities are video_device instances, either to ensure they can cast the
> entity to a struct video_device, or to count the number of video nodes
> users.
>
> The purpose of the function is thus to identify whether the media entity
> instance is an instance of the video_device object, not to check whether
> it can perform I/O. Rename it accordingly, we will introduce a more
> specific is_media_entity_v4l2_io() check when needed.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+rene...@ideasonboard.com>

Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH][davinci] ccdc_update_raw_params() frees the wrong thing

[Lad, Prabhakar]

On Wed, Jan 6, 2016 at 4:34 PM, Al Viro <v...@zeniv.linux.org.uk> wrote:
> On Tue, Jan 05, 2016 at 05:37:06PM +0000, Lad, Prabhakar wrote:
>> On Sun, Dec 13, 2015 at 12:32 AM, Al Viro <v...@zeniv.linux.org.uk> wrote:
>> > Passing a physical address to free_pages() is a bad idea.
>> > config_params->fault_pxl.fpc_table_addr is set to virt_to_phys()
>> > of __get_free_pages() return value; what we should pass to free_pages()
>> > is its phys_to_virt().  ccdc_close() does that properly, but
>> > ccdc_update_raw_params() doesn't.
>> >
>> > Signed-off-by: Al Viro <v...@zeniv.linux.org.uk>
>> >
>> Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>
>>
>> Regards,
>> --Prabhakar Lad
>
> Which tree should it go through?  I can certainly put that into
> vfs.git#work.misc, but it looks like a better fit for linux-media tree, or
> the davinci-specific one...

It needs to go linux-media tree I'll issue a pull to mauro soon.

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] media: i2c: ov2659: speedup probe if no device connected

[Lad, Prabhakar]

On Mon, Dec 21, 2015 at 1:54 PM, Grygorii Strashko
<grygorii.stras...@ti.com> wrote:
> The ov2659 driver performs device detection and initialization in the
> following way:
>  - send reset command REG_SOFTWARE_RESET
>  - load array of predefined register's setting (~150 values)
>  - read device version REG_SC_CHIP_ID_H/REG_SC_CHIP_ID_L
>  - check version and exit if invalid.
>
> As result, for not connected device there will be >~150 i2c transactions
> executed before device version checking and exit (there are no
> failures detected because ov2659 declared as I2C_CLIENT_SCCB and NACKs
> are ignored in this case).
>
> Let's fix that by checking the chip version first and start
> initialization only if it's supported.
>
> Cc: Benoit Parrot <bpar...@ti.com>
> Signed-off-by: Grygorii Strashko <grygorii.stras...@ti.com>

Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Cheers,
--Prabhakar Lad

> ---
>  drivers/media/i2c/ov2659.c | 8 +++-
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c
> index 49109f4..9b7b78c 100644
> --- a/drivers/media/i2c/ov2659.c
> +++ b/drivers/media/i2c/ov2659.c
> @@ -1321,10 +1321,6 @@ static int ov2659_detect(struct v4l2_subdev *sd)
> }
> usleep_range(1000, 2000);
>
> -   ret = ov2659_init(sd, 0);
> -   if (ret < 0)
> -   return ret;
> -
> /* Check sensor revision */
> ret = ov2659_read(client, REG_SC_CHIP_ID_H, );
> if (!ret)
> @@ -1338,8 +1334,10 @@ static int ov2659_detect(struct v4l2_subdev *sd)
> dev_err(>dev,
> "Sensor detection failed (%04X, %d)\n",
> id, ret);
> -   else
> +   else {
> dev_info(>dev, "Found OV%04X sensor\n", id);
> +   ret = ov2659_init(sd, 0);
> +   }
> }
>
> return ret;
> --
> 2.6.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] media: use unsigned for pad index

[Lad, Prabhakar]

On Fri, Dec 11, 2015 at 2:26 PM, Mauro Carvalho Chehab
<mche...@osg.samsung.com> wrote:
> The pad index is unsigned. Replace the occurences of it where
> pertinent.
>
> Suggested-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> Signed-off-by: Mauro Carvalho Chehab <mche...@osg.samsung.com>
> ---
>  drivers/staging/media/davinci_vpfe/dm365_ipipeif.c | 2 +-
>  drivers/staging/media/davinci_vpfe/dm365_isif.c| 2 +-
>  drivers/staging/media/davinci_vpfe/dm365_resizer.c | 2 +-

For the above:

Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Regards,
--Prabhakar Lad
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH][davinci] ccdc_update_raw_params() frees the wrong thing

[Lad, Prabhakar]

On Sun, Dec 13, 2015 at 12:32 AM, Al Viro <v...@zeniv.linux.org.uk> wrote:
> Passing a physical address to free_pages() is a bad idea.
> config_params->fault_pxl.fpc_table_addr is set to virt_to_phys()
> of __get_free_pages() return value; what we should pass to free_pages()
> is its phys_to_virt().  ccdc_close() does that properly, but
> ccdc_update_raw_params() doesn't.
>
> Signed-off-by: Al Viro <v...@zeniv.linux.org.uk>
>
Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Regards,
--Prabhakar Lad

> diff --git a/drivers/media/platform/davinci/dm644x_ccdc.c 
> b/drivers/media/platform/davinci/dm644x_ccdc.c
> index ffbefdf..6fba32b 100644
> --- a/drivers/media/platform/davinci/dm644x_ccdc.c
> +++ b/drivers/media/platform/davinci/dm644x_ccdc.c
> @@ -261,7 +261,7 @@ static int ccdc_update_raw_params(struct 
> ccdc_config_params_raw *raw_params)
>  */
> if (raw_params->fault_pxl.fp_num != config_params->fault_pxl.fp_num) {
> if (fpc_physaddr != NULL) {
> -   free_pages((unsigned long)fpc_physaddr,
> +   free_pages((unsigned long)fpc_virtaddr,
>get_order
>(config_params->fault_pxl.fp_num *
>FP_NUM_BYTES));
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] [media] davinci: add i2c Kconfig dependencies

[Lad, Prabhakar]

On Thu, Nov 19, 2015 at 12:59 PM, Arnd Bergmann <a...@arndb.de> wrote:
> All the davinci media drivers are using the i2c framework, and
> fail to build if that is ever disabled, e.g.:
>
> media/platform/davinci/vpif_display.c: In function 'vpif_probe':
> media/platform/davinci/vpif_display.c:1298:14: error: implicit declaration of 
> function 'i2c_get_adapter' [-Werror=implicit-function-declaration]
>
> This adds explicit Kconfig dependencies so we don't see the
> driver options if I2C is turned off.
>
> Signed-off-by: Arnd Bergmann <a...@arndb.de>

Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT PULL FOR v4.5] Davinci staging fixes

[Lad, Prabhakar]

Hi Mauro,

On Tue, Nov 17, 2015 at 4:29 PM, Mauro Carvalho Chehab
 wrote:
> Em Mon, 09 Nov 2015 23:27:02 +0200
> Laurent Pinchart  escreveu:
>
>> Hi Mauro,
>>
>> I've collected the pending Davinci staging fixes from patchwork and prepared 
>> a
>> branch for you.
>>
>> Prabhakar, is that fine with you ? Do you still maintain the driver ? If so,
>> do you expect patches to be picked up when you ack them, or can you collect
>> them in a branch somewhere and send a pull request ?
>
> As the patches on this tree are trivial, and one of them has Prabhakar's ack,
> I'll apply them, as I'm assuming that they're all ok for Prabhakar.
>
Somehow this mail got buried into my inbox, If Laurent can pick it that’s fine
with me Or else Ill create a pull request for you.

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] [media] i2c: constify v4l2_ctrl_ops structures

[Lad, Prabhakar]

On Fri, Nov 13, 2015 at 10:05 PM, Julia Lawall <julia.law...@lip6.fr> wrote:
> These v4l2_ctrl_ops structures are never modified, like all the other
> v4l2_ctrl_ops structures, so declare them as const.
>
> Done with the help of Coccinelle.
>
> Signed-off-by: Julia Lawall <julia.law...@lip6.fr>
>

Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] [media] tvp5150: add support for asynchronous probing

[Lad, Prabhakar]

On Mon, Sep 21, 2015 at 12:23 PM, Javier Martinez Canillas
<jav...@osg.samsung.com> wrote:
> Allow the subdevice to be probed asynchronously.
>
> Signed-off-by: Javier Martinez Canillas <jav...@osg.samsung.com>
>
Acked-by: Lad, Prabhakar <prabhakar.cse...@gmail.com>

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: drivers/media/platform/am437x/am437x-vpfe.c:1698: bad test ?

[Lad, Prabhakar]

Hi David,

On Mon, Aug 3, 2015 at 3:02 PM, David Binderman dcb...@hotmail.com wrote:
 Hello there,

 drivers/media/platform/am437x/am437x-vpfe.c:1698:27: warning: self-comparison 
 always evaluates to true [-Wtautological-compare]

  if (client-addr == curr_client-addr 
 client-adapter-nr == client-adapter-nr) {

 maybe

  if (client-addr == curr_client-addr 
 client-adapter-nr == curr_client-adapter-nr) {

Good catch!
I'll post a patch fixing it.

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Patch v3 1/1] media: am437x-vpfe: Fix a race condition during release

[Lad, Prabhakar]

On Mon, Jun 29, 2015 at 10:18 PM, Benoit Parrot bpar...@ti.com wrote:
 There was a race condition where during cleanup/release operation
 on-going streaming would cause a kernel panic because the hardware
 module was disabled prematurely with IRQ still pending.

 Fixes: 417d2e507edc ([media] media: platform: add VPFE capture driver 
 support for AM437X)
 Cc: sta...@vger.kernel.org # v4.0+
 Signed-off-by: Benoit Parrot bpar...@ti.com

Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Cheers,
--Prabhakar Lad

 ---
 Changes since v2:
 - fix the stable commit reference syntax

  drivers/media/platform/am437x/am437x-vpfe.c | 11 +--
  1 file changed, 9 insertions(+), 2 deletions(-)

 diff --git a/drivers/media/platform/am437x/am437x-vpfe.c 
 b/drivers/media/platform/am437x/am437x-vpfe.c
 index a30cc2f7e4f1..eb25c43da126 100644
 --- a/drivers/media/platform/am437x/am437x-vpfe.c
 +++ b/drivers/media/platform/am437x/am437x-vpfe.c
 @@ -1185,14 +1185,21 @@ static int vpfe_initialize_device(struct vpfe_device 
 *vpfe)
  static int vpfe_release(struct file *file)
  {
 struct vpfe_device *vpfe = video_drvdata(file);
 +   bool fh_singular = v4l2_fh_is_singular_file(file);
 int ret;

 mutex_lock(vpfe-lock);

 -   if (v4l2_fh_is_singular_file(file))
 -   vpfe_ccdc_close(vpfe-ccdc, vpfe-pdev);
 +   /* the release helper will cleanup any on-going streaming */
 ret = _vb2_fop_release(file, NULL);

 +   /*
 +* If this was the last open file.
 +* Then de-initialize hw module.
 +*/
 +   if (fh_singular)
 +   vpfe_ccdc_close(vpfe-ccdc, vpfe-pdev);
 +
 mutex_unlock(vpfe-lock);

 return ret;
 --
 1.8.5.1

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Patch v3 1/1] media: am437x-vpfe: Requested frame size and fmt overwritten by current sensor setting

[Lad, Prabhakar]

On Mon, Jun 29, 2015 at 10:19 PM, Benoit Parrot bpar...@ti.com wrote:
 Upon a S_FMT the input/requested frame size and pixel format is
 overwritten by the current sub-device settings.
 Fix this so application can actually set the frame size and format.

 Fixes: 417d2e507edc ([media] media: platform: add VPFE capture driver 
 support for AM437X)
 Cc: sta...@vger.kernel.org # v4.0+
 Signed-off-by: Benoit Parrot bpar...@ti.com

Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Cheers,
--Prabhakar Lad

 ---
 Changes since v2:
 - fix the stable commit reference syntax

  drivers/media/platform/am437x/am437x-vpfe.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/drivers/media/platform/am437x/am437x-vpfe.c 
 b/drivers/media/platform/am437x/am437x-vpfe.c
 index eb25c43da126..0fa62c50f62d 100644
 --- a/drivers/media/platform/am437x/am437x-vpfe.c
 +++ b/drivers/media/platform/am437x/am437x-vpfe.c
 @@ -1584,7 +1584,7 @@ static int vpfe_s_fmt(struct file *file, void *priv,
 return -EBUSY;
 }

 -   ret = vpfe_try_fmt(file, priv, fmt);
 +   ret = vpfe_try_fmt(file, priv, format);
 if (ret)
 return ret;

 --
 1.8.5.1

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 08/12] media/i2c/tvp514x: Remove compat control ops

[Lad, Prabhakar]

On Fri, Jun 12, 2015 at 5:31 PM, Ricardo Ribalda Delgado
ricardo.riba...@gmail.com wrote:
 They are no longer used in old non-control-framework
 bridge drivers.

 Reported-by: Hans Verkuil hans.verk...@cisco.com
 Signed-off-by: Ricardo Ribalda Delgado ricardo.riba...@gmail.com

Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Cheers,
--Prabhakar Lad

 ---
  drivers/media/i2c/tvp514x.c | 11 ---
  1 file changed, 11 deletions(-)

 diff --git a/drivers/media/i2c/tvp514x.c b/drivers/media/i2c/tvp514x.c
 index 24e47279e30c..a93985a9b070 100644
 --- a/drivers/media/i2c/tvp514x.c
 +++ b/drivers/media/i2c/tvp514x.c
 @@ -957,16 +957,6 @@ static int tvp514x_set_pad_format(struct v4l2_subdev *sd,
 return 0;
  }

 -static const struct v4l2_subdev_core_ops tvp514x_core_ops = {
 -   .g_ext_ctrls = v4l2_subdev_g_ext_ctrls,
 -   .try_ext_ctrls = v4l2_subdev_try_ext_ctrls,
 -   .s_ext_ctrls = v4l2_subdev_s_ext_ctrls,
 -   .g_ctrl = v4l2_subdev_g_ctrl,
 -   .s_ctrl = v4l2_subdev_s_ctrl,
 -   .queryctrl = v4l2_subdev_queryctrl,
 -   .querymenu = v4l2_subdev_querymenu,
 -};
 -
  static const struct v4l2_subdev_video_ops tvp514x_video_ops = {
 .s_std = tvp514x_s_std,
 .s_routing = tvp514x_s_routing,
 @@ -983,7 +973,6 @@ static const struct v4l2_subdev_pad_ops tvp514x_pad_ops = 
 {
  };

  static const struct v4l2_subdev_ops tvp514x_ops = {
 -   .core = tvp514x_core_ops,
 .video = tvp514x_video_ops,
 .pad = tvp514x_pad_ops,
  };
 --
 2.1.4

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 01/12] media/i2c/adv7343: Remove compat control ops

[Lad, Prabhakar]

On Fri, Jun 12, 2015 at 5:31 PM, Ricardo Ribalda Delgado
ricardo.riba...@gmail.com wrote:
 They are no longer used in old non-control-framework
 bridge drivers.

 Reported-by: Hans Verkuil hans.verk...@cisco.com
 Signed-off-by: Ricardo Ribalda Delgado ricardo.riba...@gmail.com

Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Cheers,
--Prabhakar Lad

 ---
  drivers/media/i2c/adv7343.c | 7 ---
  1 file changed, 7 deletions(-)

 diff --git a/drivers/media/i2c/adv7343.c b/drivers/media/i2c/adv7343.c
 index 7c50833e7d17..d27283135490 100644
 --- a/drivers/media/i2c/adv7343.c
 +++ b/drivers/media/i2c/adv7343.c
 @@ -319,13 +319,6 @@ static const struct v4l2_ctrl_ops adv7343_ctrl_ops = {

  static const struct v4l2_subdev_core_ops adv7343_core_ops = {
 .log_status = adv7343_log_status,
 -   .g_ext_ctrls = v4l2_subdev_g_ext_ctrls,
 -   .try_ext_ctrls = v4l2_subdev_try_ext_ctrls,
 -   .s_ext_ctrls = v4l2_subdev_s_ext_ctrls,
 -   .g_ctrl = v4l2_subdev_g_ctrl,
 -   .s_ctrl = v4l2_subdev_s_ctrl,
 -   .queryctrl = v4l2_subdev_queryctrl,
 -   .querymenu = v4l2_subdev_querymenu,
  };

  static int adv7343_s_std_output(struct v4l2_subdev *sd, v4l2_std_id std)
 --
 2.1.4

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 09/12] media/i2c/tvp7002: Remove compat control ops

[Lad, Prabhakar]

On Fri, Jun 12, 2015 at 5:31 PM, Ricardo Ribalda Delgado
ricardo.riba...@gmail.com wrote:
 They are no longer used in old non-control-framework
 bridge drivers.

 Reported-by: Hans Verkuil hans.verk...@cisco.com
 Signed-off-by: Ricardo Ribalda Delgado ricardo.riba...@gmail.com

Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Cheers,
--Prabhakar Lad

 ---
  drivers/media/i2c/tvp7002.c | 7 ---
  1 file changed, 7 deletions(-)

 diff --git a/drivers/media/i2c/tvp7002.c b/drivers/media/i2c/tvp7002.c
 index 05077cffd235..f617d8b745ee 100644
 --- a/drivers/media/i2c/tvp7002.c
 +++ b/drivers/media/i2c/tvp7002.c
 @@ -861,13 +861,6 @@ tvp7002_set_pad_format(struct v4l2_subdev *sd, struct 
 v4l2_subdev_pad_config *cf
  /* V4L2 core operation handlers */
  static const struct v4l2_subdev_core_ops tvp7002_core_ops = {
 .log_status = tvp7002_log_status,
 -   .g_ext_ctrls = v4l2_subdev_g_ext_ctrls,
 -   .try_ext_ctrls = v4l2_subdev_try_ext_ctrls,
 -   .s_ext_ctrls = v4l2_subdev_s_ext_ctrls,
 -   .g_ctrl = v4l2_subdev_g_ctrl,
 -   .s_ctrl = v4l2_subdev_s_ctrl,
 -   .queryctrl = v4l2_subdev_queryctrl,
 -   .querymenu = v4l2_subdev_querymenu,
  #ifdef CONFIG_VIDEO_ADV_DEBUG
 .g_register = tvp7002_g_register,
 .s_register = tvp7002_s_register,
 --
 2.1.4

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] media: davinci: vpbe: use v4l2_get_timestamp()

[Lad Prabhakar]

From: Lad, Prabhakar prabhakar.cse...@gmail.com

this patch makes use of helper function v4l2_get_timestamp()
to set the timestamp of vb2 buffer.

Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
---
 drivers/media/platform/davinci/vpbe_display.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/media/platform/davinci/vpbe_display.c 
b/drivers/media/platform/davinci/vpbe_display.c
index c4ab46f..f69cdd7 100644
--- a/drivers/media/platform/davinci/vpbe_display.c
+++ b/drivers/media/platform/davinci/vpbe_display.c
@@ -71,15 +71,10 @@ static int venc_is_second_field(struct vpbe_display 
*disp_dev)
 static void vpbe_isr_even_field(struct vpbe_display *disp_obj,
struct vpbe_layer *layer)
 {
-   struct timespec timevalue;
-
if (layer-cur_frm == layer-next_frm)
return;
-   ktime_get_ts(timevalue);
-   layer-cur_frm-vb.v4l2_buf.timestamp.tv_sec =
-   timevalue.tv_sec;
-   layer-cur_frm-vb.v4l2_buf.timestamp.tv_usec =
-   timevalue.tv_nsec / NSEC_PER_USEC;
+
+   v4l2_get_timestamp(layer-cur_frm-vb.v4l2_buf.timestamp);
vb2_buffer_done(layer-cur_frm-vb, VB2_BUF_STATE_DONE);
/* Make cur_frm pointing to next_frm */
layer-cur_frm = layer-next_frm;
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 0/3] davinci_vpfe: minor cleanups

[Lad Prabhakar]

From: Lad, Prabhakar prabhakar.cse...@gmail.com

Hi Hans,

These patch series includes minor cleanups for davinci vpfe
driver.

Lad, Prabhakar (3):
  media: davinci_vpfe: clear the output_specs
  media: davinci_vpfe: set minimum required buffers to three
  media: davinci_vpfe: use monotonic timestamp

 drivers/staging/media/davinci_vpfe/dm365_resizer.c   |  1 +
 drivers/staging/media/davinci_vpfe/vpfe_mc_capture.h |  2 --
 drivers/staging/media/davinci_vpfe/vpfe_video.c  | 18 +-
 3 files changed, 6 insertions(+), 15 deletions(-)

-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] media: davinci_vpfe: clear the output_specs

[Lad Prabhakar]

From: Lad, Prabhakar prabhakar.cse...@gmail.com

clear of the output_specs before passing it to the
configure_resizer_out_params(), so that no garbage values
are set.

This fixes following build warning:
drivers/staging/media/davinci_vpfe/dm365_resizer.c: In function 
'resizer_set_stream':
drivers/staging/media/davinci_vpfe/dm365_resizer.c:190:46: warning: 
'output_specs.vst_c'
may be used uninitialized in this function [-Wmaybe-uninitialized]
  param-ext_mem_param[index].rsz_sdr_ptr_s_c = output-vst_c;
  ^
drivers/staging/media/davinci_vpfe/dm365_resizer.c:316:30: note: 
'output_specs.vst_c' was declared here
  struct vpfe_rsz_output_spec output_specs;
  ^
Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
---
 drivers/staging/media/davinci_vpfe/dm365_resizer.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/media/davinci_vpfe/dm365_resizer.c 
b/drivers/staging/media/davinci_vpfe/dm365_resizer.c
index b649813..acb293e 100644
--- a/drivers/staging/media/davinci_vpfe/dm365_resizer.c
+++ b/drivers/staging/media/davinci_vpfe/dm365_resizer.c
@@ -321,6 +321,7 @@ static int resizer_configure_output_win(struct 
vpfe_resizer_device *resizer)
 
outformat = resizer-resizer_a.formats[RESIZER_PAD_SOURCE];
 
+   memset(output_specs, 0x0, sizeof(struct vpfe_rsz_output_spec));
output_specs.vst_y = param-user_config.vst;
if (outformat-code == MEDIA_BUS_FMT_YDYUYDYV8_1X16)
output_specs.vst_c = param-user_config.vst;
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] media: davinci_vpfe: use monotonic timestamp

[Lad Prabhakar]

From: Lad, Prabhakar prabhakar.cse...@gmail.com

V4L2 drivers should use MONOTONIC timestamps instead of gettimeofday,
which is affected by daylight savings time.

Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
---
 drivers/staging/media/davinci_vpfe/vpfe_video.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c 
b/drivers/staging/media/davinci_vpfe/vpfe_video.c
index 6744192..87048a1 100644
--- a/drivers/staging/media/davinci_vpfe/vpfe_video.c
+++ b/drivers/staging/media/davinci_vpfe/vpfe_video.c
@@ -470,7 +470,7 @@ void vpfe_video_process_buffer_complete(struct 
vpfe_video_device *video)
 {
struct vpfe_pipeline *pipe = video-pipe;
 
-   do_gettimeofday(video-cur_frm-vb.v4l2_buf.timestamp);
+   v4l2_get_timestamp(video-cur_frm-vb.v4l2_buf.timestamp);
vb2_buffer_done(video-cur_frm-vb, VB2_BUF_STATE_DONE);
if (pipe-state == VPFE_PIPELINE_STREAM_CONTINUOUS)
video-cur_frm = video-next_frm;
@@ -1337,6 +1337,7 @@ static int vpfe_reqbufs(struct file *file, void *priv,
q-ops = video_qops;
q-mem_ops = vb2_dma_contig_memops;
q-buf_struct_size = sizeof(struct vpfe_cap_buffer);
+   q-timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 
ret = vb2_queue_init(q);
if (ret) {
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] media: davinci_vpfe: set minimum required buffers to three

[Lad Prabhakar]

From: Lad, Prabhakar prabhakar.cse...@gmail.com

this patch sets nbuffers to three or more and drops the
unset member video_limit which just a copy paste from
earlier driver.

Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
---
 drivers/staging/media/davinci_vpfe/vpfe_mc_capture.h |  2 --
 drivers/staging/media/davinci_vpfe/vpfe_video.c  | 15 +++
 2 files changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/media/davinci_vpfe/vpfe_mc_capture.h 
b/drivers/staging/media/davinci_vpfe/vpfe_mc_capture.h
index 2632a80..8ad8d74 100644
--- a/drivers/staging/media/davinci_vpfe/vpfe_mc_capture.h
+++ b/drivers/staging/media/davinci_vpfe/vpfe_mc_capture.h
@@ -67,8 +67,6 @@ struct vpfe_device {
/* CCDC IRQs used when CCDC/ISIF output to SDRAM */
unsigned intccdc_irq0;
unsigned intccdc_irq1;
-   /* maximum video memory that is available*/
-   unsigned intvideo_limit;
/* media device */
struct media_device media_dev;
/* ccdc subdevice */
diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c 
b/drivers/staging/media/davinci_vpfe/vpfe_video.c
index 06d48d5..6744192 100644
--- a/drivers/staging/media/davinci_vpfe/vpfe_video.c
+++ b/drivers/staging/media/davinci_vpfe/vpfe_video.c
@@ -27,9 +27,6 @@
 #include vpfe.h
 #include vpfe_mc_capture.h
 
-/* minimum number of buffers needed in cont-mode */
-#define MIN_NUM_BUFFERS3
-
 static int debug;
 
 /* get v4l2 subdev pointer to external subdev which is active */
@@ -1088,20 +1085,14 @@ vpfe_buffer_queue_setup(struct vb2_queue *vq, const 
struct v4l2_format *fmt,
struct vpfe_fh *fh = vb2_get_drv_priv(vq);
struct vpfe_video_device *video = fh-video;
struct vpfe_device *vpfe_dev = video-vpfe_dev;
-   struct vpfe_pipeline *pipe = video-pipe;
unsigned long size;
 
v4l2_dbg(1, debug, vpfe_dev-v4l2_dev, vpfe_buffer_queue_setup\n);
size = video-fmt.fmt.pix.sizeimage;
 
-   if (vpfe_dev-video_limit) {
-   while (size * *nbuffers  vpfe_dev-video_limit)
-   (*nbuffers)--;
-   }
-   if (pipe-state == VPFE_PIPELINE_STREAM_CONTINUOUS) {
-   if (*nbuffers  MIN_NUM_BUFFERS)
-   *nbuffers = MIN_NUM_BUFFERS;
-   }
+   if (vq-num_buffers + *nbuffers  3)
+   *nbuffers = 3 - vq-num_buffers;
+
*nplanes = 1;
sizes[0] = size;
alloc_ctxs[0] = video-alloc_ctx;
-- 
2.1.4

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] usb drivers: use BUG_ON() instead of if () BUG

[Lad, Prabhakar]

On Tue, May 19, 2015 at 12:00 PM, Mauro Carvalho Chehab
mche...@osg.samsung.com wrote:
 Some USB drivers have a logic at the VB buffer handling like:
 if (in_interrupt())
 BUG();
 Use, instead:
 BUG_ON(in_interrupt());

 Btw, this logic looks weird on my eyes. We should convert them
 to use VB2, in order to avoid those crappy things.

 Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com

Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] [media] ov2659: add v4l2_subdev dependency

[Lad, Prabhakar]

Hi Arnd,

Thanks for the patch.

On Tue, May 19, 2015 at 1:39 PM, Arnd Bergmann a...@arndb.de wrote:
 The newly added ov2659 driver uses the v4l2 subdev API, but
 can be enabled even when that API is not part of the kernel,
 resulting in this build error:

 media/i2c/ov2659.c: In function 'ov2659_get_fmt':
 media/i2c/ov2659.c:1054:8: error: implicit declaration of function 
 'v4l2_subdev_get_try_format' [-Werror=implicit-function-declaration]
 media/i2c/ov2659.c:1054:6: warning: assignment makes pointer from integer 
 without a cast [-Wint-conversion]
 media/i2c/ov2659.c: In function 'ov2659_set_fmt':
 media/i2c/ov2659.c:1129:6: warning: assignment makes pointer from integer 
 without a cast [-Wint-conversion]
 media/i2c/ov2659.c: In function 'ov2659_open':
 media/i2c/ov2659.c:1264:38: error: 'struct v4l2_subdev_fh' has no member 
 named 'pad'

 This adds an explicit dependency, like all the other drivers have.

Patch fixing the above issue is already posted in the ML [1].

[1] https://patchwork.linuxtv.org/patch/29665/

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] media: v4l2-core/v4l2-of.c: determine bus_type only on hsync/vsync flags

[Lad Prabhakar]

From: Lad, Prabhakar prabhakar.cse...@gmail.com

the bus_type needs to be determined only on the hsync/vsync flags,
this patch fixes the above by moving the check just after hsync/vsync
flags are being set.

Reported-by: Nikhil Devshatwar nikhil...@ti.com
Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
---
 drivers/media/v4l2-core/v4l2-of.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-of.c 
b/drivers/media/v4l2-core/v4l2-of.c
index c52fb96..7f89c70 100644
--- a/drivers/media/v4l2-core/v4l2-of.c
+++ b/drivers/media/v4l2-core/v4l2-of.c
@@ -93,6 +93,11 @@ static void v4l2_of_parse_parallel_bus(const struct 
device_node *node,
flags |= v ? V4L2_MBUS_VSYNC_ACTIVE_HIGH :
V4L2_MBUS_VSYNC_ACTIVE_LOW;
 
+   if (flags)
+   endpoint-bus_type = V4L2_MBUS_PARALLEL;
+   else
+   endpoint-bus_type = V4L2_MBUS_BT656;
+
if (!of_property_read_u32(node, pclk-sample, v))
flags |= v ? V4L2_MBUS_PCLK_SAMPLE_RISING :
V4L2_MBUS_PCLK_SAMPLE_FALLING;
@@ -100,10 +105,6 @@ static void v4l2_of_parse_parallel_bus(const struct 
device_node *node,
if (!of_property_read_u32(node, field-even-active, v))
flags |= v ? V4L2_MBUS_FIELD_EVEN_HIGH :
V4L2_MBUS_FIELD_EVEN_LOW;
-   if (flags)
-   endpoint-bus_type = V4L2_MBUS_PARALLEL;
-   else
-   endpoint-bus_type = V4L2_MBUS_BT656;
 
if (!of_property_read_u32(node, data-active, v))
flags |= v ? V4L2_MBUS_DATA_ACTIVE_HIGH :
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ov2659: Don't depend on subdev API

[Lad, Prabhakar]

Hi Mauro,

Thanks for the patch.

On Thu, May 14, 2015 at 11:27 PM, Mauro Carvalho Chehab
mche...@osg.samsung.com wrote:
 The subdev API is optional. No driver should depend on it.

 Avoid compilation breakages if subdev API is not selected:

 drivers/media/i2c/ov2659.c: In function ‘ov2659_get_fmt’:
 drivers/media/i2c/ov2659.c:1054:3: error: implicit declaration of function 
 ‘v4l2_subdev_get_try_format’ [-Werror=implicit-function-declaration]
mf = v4l2_subdev_get_try_format(sd, cfg, 0);
^
 drivers/media/i2c/ov2659.c:1054:6: warning: assignment makes pointer from 
 integer without a cast
mf = v4l2_subdev_get_try_format(sd, cfg, 0);
   ^
 drivers/media/i2c/ov2659.c: In function ‘ov2659_set_fmt’:
 drivers/media/i2c/ov2659.c:1129:6: warning: assignment makes pointer from 
 integer without a cast
mf = v4l2_subdev_get_try_format(sd, cfg, fmt-pad);
   ^
 drivers/media/i2c/ov2659.c: In function ‘ov2659_open’:
 drivers/media/i2c/ov2659.c:1264:38: error: ‘struct v4l2_subdev_fh’ has no 
 member named ‘pad’
  v4l2_subdev_get_try_format(sd, fh-pad, 0);
   ^

 Compile-tested only.

 Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com

Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com
Tested-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Clarify expression which uses both multiplication and pointer dereference

[Lad, Prabhakar]

On Mon, May 18, 2015 at 7:51 AM, Michal Kubecek mkube...@suse.cz wrote:
 On Sun, May 17, 2015 at 07:18:42PM +0200, Alex Dowad wrote:
 This fixes a checkpatch style error in vpfe_buffer_queue_setup.

 Signed-off-by: Alex Dowad alexinbeij...@gmail.com
 ---
  drivers/staging/media/davinci_vpfe/vpfe_video.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c 
 b/drivers/staging/media/davinci_vpfe/vpfe_video.c
 index 06d48d5..04a687c 100644
 --- a/drivers/staging/media/davinci_vpfe/vpfe_video.c
 +++ b/drivers/staging/media/davinci_vpfe/vpfe_video.c
 @@ -1095,7 +1095,7 @@ vpfe_buffer_queue_setup(struct vb2_queue *vq, const 
 struct v4l2_format *fmt,
   size = video-fmt.fmt.pix.sizeimage;

   if (vpfe_dev-video_limit) {
 - while (size * *nbuffers  vpfe_dev-video_limit)
 + while (size * (*nbuffers)  vpfe_dev-video_limit)
   (*nbuffers)--;
   }
   if (pipe-state == VPFE_PIPELINE_STREAM_CONTINUOUS) {

 Style issue aside, is there a reason not to use

 if (size * *nbuffers  vpfe_dev-video_limit)
 *nbuffers = vpfe_dev-video_limit / size;

 instead?

I would prefer this.

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Clarify expression which uses both multiplication and pointer dereference

[Lad, Prabhakar]

Hi Hans,

On Mon, May 18, 2015 at 9:15 AM, Hans Verkuil hverk...@xs4all.nl wrote:


 On 05/18/2015 10:06 AM, Lad, Prabhakar wrote:
 On Mon, May 18, 2015 at 7:51 AM, Michal Kubecek mkube...@suse.cz wrote:
 On Sun, May 17, 2015 at 07:18:42PM +0200, Alex Dowad wrote:
 This fixes a checkpatch style error in vpfe_buffer_queue_setup.

 Signed-off-by: Alex Dowad alexinbeij...@gmail.com
 ---
  drivers/staging/media/davinci_vpfe/vpfe_video.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c 
 b/drivers/staging/media/davinci_vpfe/vpfe_video.c
 index 06d48d5..04a687c 100644
 --- a/drivers/staging/media/davinci_vpfe/vpfe_video.c
 +++ b/drivers/staging/media/davinci_vpfe/vpfe_video.c
 @@ -1095,7 +1095,7 @@ vpfe_buffer_queue_setup(struct vb2_queue *vq, const 
 struct v4l2_format *fmt,
   size = video-fmt.fmt.pix.sizeimage;

   if (vpfe_dev-video_limit) {
 - while (size * *nbuffers  vpfe_dev-video_limit)
 + while (size * (*nbuffers)  vpfe_dev-video_limit)
   (*nbuffers)--;
   }
   if (pipe-state == VPFE_PIPELINE_STREAM_CONTINUOUS) {

 Style issue aside, is there a reason not to use

 if (size * *nbuffers  vpfe_dev-video_limit)
 *nbuffers = vpfe_dev-video_limit / size;

 instead?

 I would prefer this.

 As far as I can see video_limit is never set at all, so this code (and the 
 video_limit
 field) can just be removed.

 I think this is a left-over from old code, long since removed.

Yes makes sense, I'll fix it up and post a patch for it.

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 00/17] media: blackfin: bfin_capture enhancements

[Lad, Prabhakar]

Hi Scott,

On Tue, May 5, 2015 at 10:58 AM, Scott Jiang
scott.jiang.li...@gmail.com wrote:
 2015-04-24 18:04 GMT+08:00 Hans Verkuil hverk...@xs4all.nl:
 On 04/10/2015 12:42 PM, Scott Jiang wrote:
 Hi Hans,


 Hans, I tried to use v4l2-compliance but it failed to compile. Sorry
 for telling you it have passed compilation because I forgot to use
 blackfin toolchain.
 ./configure --without-jpeg  --host=bfin-linux-uclibc --disable-libv4l

 The main problem is there is no argp.h in uClibc, how to disable checking 
 this?

 checking for argp.h... no
 configure: error: Cannot continue: argp.h not found

 Scott


 Hi Scott,

 Can you try this patch for v4l-utils? It makes argp optional, and it should
 allow v4l2-compliance to compile with uclibc (unless there are more 
 problems).

 I'm no autoconf guru, so I'm not certain if everything is correct, but it
 seemed to do its job when I remove argp.h from my system.


 Yes, I can pass configure now. But there is another error when make

 make[3]: Entering directory
 `/home/scott/projects/git-kernel/v4l-utils/lib/libdvbv5'
   CC libdvbv5_la-parse_string.lo
 parse_string.c:26:19: error: iconv.h: No such file or directory
 parse_string.c: In function 'dvb_iconv_to_charset':
 parse_string.c:316: error: 'iconv_t' undeclared (first use in this function)

 I tried to pass this library, while --without-libdvbv5 is not supported.


 If you can pass the configure step, then you should be able to run this:

 cd utils/v4l2-compliance
 cat *.cpp x.cpp
 g++ -o v4l2-compliance x.cpp -I . -I ../../include/ -DNO_LIBV4L2

 (you need to use the right toolchain here, of course)

 If this compiles OK, then you have a v4l2-compliance tool that you can
 use.

 Yes, this method works. The test results of v4l2-compliance are below,
 I'm sorry the kernel has not upgraded to 4.0.
 root:/ ./v4l2-compliance -d 0

v4l2-compliance with -s option would interesting to watch.

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 09/14] zoran: fix indent

[Lad, Prabhakar]

On Tue, Apr 28, 2015 at 4:43 PM, Mauro Carvalho Chehab
mche...@osg.samsung.com wrote:
 As reported by smatch:
 drivers/media/pci/zoran/zoran_device.c:1594 zoran_init_hardware() 
 warn: inconsistent indenting

 Fix indent. While here, fix CodingStyle and remove dead code, as it
 can always be recovered from git logs.

 Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com

Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 18/27] cx25840: fix bad identing

[Lad, Prabhakar]

On Thu, Apr 30, 2015 at 12:06 AM, Mauro Carvalho Chehab
mche...@osg.samsung.com wrote:
 drivers/media/i2c/cx25840/cx25840-core.c:974 input_change() warn: 
 inconsistent indenting

 Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com

Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] am437x-vpfe: really update the vpfe_ccdc_update_raw_params data

[Lad, Prabhakar]

On Tue, Apr 28, 2015 at 12:59 PM, Mauro Carvalho Chehab
mche...@osg.samsung.com wrote:
 drivers/media/platform/am437x/am437x-vpfe.c: In function 
 'vpfe_ccdc_update_raw_params':
 drivers/media/platform/am437x/am437x-vpfe.c:430:38: warning: variable 
 'config_params' set but not used [-Wunused-but-set-variable]
   struct vpfe_ccdc_config_params_raw *config_params =
   ^

 vpfe_ccdc_update_raw_params() is supposed to update the raw
 params at ccdc. However, it is just creating a local var and changing
 it.

 Compile-tested only.

 Cc: Benoit Parrot bpar...@ti.com
 Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com

Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] am437x: Fix a wrong identation

[Lad, Prabhakar]

On Tue, Apr 28, 2015 at 12:59 PM, Mauro Carvalho Chehab
mche...@osg.samsung.com wrote:
 drivers/media/platform/am437x/am437x-vpfe.c:513 vpfe_ccdc_set_params() warn: 
 inconsistent indenting

 Cc: Benoit Parrot bpar...@ti.com
 Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com

Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] am437x: remove unused variable

[Lad, Prabhakar]

On Tue, Apr 28, 2015 at 12:59 PM, Mauro Carvalho Chehab
mche...@osg.samsung.com wrote:
 drivers/media/platform/am437x/am437x-vpfe.c: In function 
 'vpfe_get_subdev_input_index':
 drivers/media/platform/am437x/am437x-vpfe.c:1679:27: warning: variable 
 'sdinfo' set but not used [-Wunused-but-set-variable]
   struct vpfe_subdev_info *sdinfo;
^

 Cc: Benoit Parrot bpar...@ti.com
 Signed-off-by: Mauro Carvalho Chehab mche...@osg.samsung.com

Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [media] i2c: ov2659: signedness bug inov2659_set_fmt()

[Lad, Prabhakar]

On Wed, Apr 15, 2015 at 8:12 PM, Dan Carpenter dan.carpen...@oracle.com wrote:
 This needs to be signed or there is a risk of hitting a forever loop.

 Fixes: c4c0283ab3cd ('[media] media: i2c: add support for omnivision's ov2659 
 sensor')
 Signed-off-by: Dan Carpenter dan.carpen...@oracle.com

Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Cheers,
--Prabhakar Lad

 diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c
 index edebd11..d700a1d 100644
 --- a/drivers/media/i2c/ov2659.c
 +++ b/drivers/media/i2c/ov2659.c
 @@ -1102,7 +1102,7 @@ static int ov2659_set_fmt(struct v4l2_subdev *sd,
   struct v4l2_subdev_format *fmt)
  {
 struct i2c_client *client = v4l2_get_subdevdata(sd);
 -   unsigned int index = ARRAY_SIZE(ov2659_formats);
 +   int index = ARRAY_SIZE(ov2659_formats);
 struct v4l2_mbus_framefmt *mf = fmt-format;
 const struct ov2659_framesize *size = NULL;
 struct ov2659 *ov2659 = to_ov2659(sd);
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/7] v4l2: replace enum_mbus_fmt by enum_mbus_code

[Lad, Prabhakar]

Hi Hans,

Thanks for the patch.

On Thu, Apr 9, 2015 at 11:21 AM, Hans Verkuil hverk...@xs4all.nl wrote:
 From: Hans Verkuil hans.verk...@cisco.com

 Replace all calls to the enum_mbus_fmt video op by the pad
 enum_mbus_code op and remove the duplicate video op.

 Signed-off-by: Hans Verkuil hans.verk...@cisco.com
 Cc: Guennadi Liakhovetski g.liakhovet...@gmx.de
 Cc: Scott Jiang scott.jiang.li...@gmail.com
 Cc: Jonathan Corbet cor...@lwn.net
 Cc: Kamil Debski k.deb...@samsung.com
 ---
  drivers/media/i2c/adv7170.c| 15 
  drivers/media/i2c/adv7175.c| 15 
  drivers/media/i2c/adv7183.c| 15 
  drivers/media/i2c/adv7842.c| 11 +
  drivers/media/i2c/ak881x.c | 15 
  drivers/media/i2c/ml86v7667.c  | 15 
  drivers/media/i2c/mt9v011.c| 15 
  drivers/media/i2c/ov7670.c | 11 +
  drivers/media/i2c/soc_camera/imx074.c  | 16 +
  drivers/media/i2c/soc_camera/mt9m001.c | 15 
  drivers/media/i2c/soc_camera/mt9m111.c | 15 
  drivers/media/i2c/soc_camera/mt9t031.c | 15 
  drivers/media/i2c/soc_camera/mt9t112.c | 15 
  drivers/media/i2c/soc_camera/mt9v022.c | 15 
  drivers/media/i2c/soc_camera/ov2640.c  | 15 
  drivers/media/i2c/soc_camera/ov5642.c  | 15 
  drivers/media/i2c/soc_camera/ov6650.c  | 15 
  drivers/media/i2c/soc_camera/ov772x.c  | 15 
  drivers/media/i2c/soc_camera/ov9640.c  | 15 
  drivers/media/i2c/soc_camera/ov9740.c  | 19 +--
  drivers/media/i2c/soc_camera/rj54n1cb0c.c  | 15 
  drivers/media/i2c/soc_camera/tw9910.c  | 15 
  drivers/media/i2c/sr030pc30.c  | 16 +
  drivers/media/i2c/tvp514x.c| 20 
  drivers/media/i2c/tvp7002.c| 20 

For the above 2,

Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Cheers,
--Prabhakar Lad

  drivers/media/i2c/vs6624.c | 15 
  drivers/media/platform/blackfin/bfin_capture.c | 17 +-
  drivers/media/platform/soc_camera/atmel-isi.c  | 19 ---
  drivers/media/platform/soc_camera/mx2_camera.c | 27 
 --
  drivers/media/platform/soc_camera/mx3_camera.c | 23 ++
  drivers/media/platform/soc_camera/omap1_camera.c   | 21 +
  drivers/media/platform/soc_camera/pxa_camera.c | 19 ---
  drivers/media/platform/soc_camera/rcar_vin.c   | 19 ---
  .../platform/soc_camera/sh_mobile_ceu_camera.c | 19 ---
  drivers/media/platform/soc_camera/soc_camera.c | 15 
  .../platform/soc_camera/soc_camera_platform.c  | 15 
  include/media/v4l2-subdev.h|  4 
  38 files changed, 361 insertions(+), 250 deletions(-)

 diff --git a/drivers/media/i2c/adv7170.c b/drivers/media/i2c/adv7170.c
 index 40a1a95..cfe963b 100644
 --- a/drivers/media/i2c/adv7170.c
 +++ b/drivers/media/i2c/adv7170.c
 @@ -262,13 +262,14 @@ static int adv7170_s_routing(struct v4l2_subdev *sd,
 return 0;
  }

 -static int adv7170_enum_fmt(struct v4l2_subdev *sd, unsigned int index,
 -   u32 *code)
 +static int adv7170_enum_mbus_code(struct v4l2_subdev *sd,
 +   struct v4l2_subdev_pad_config *cfg,
 +   struct v4l2_subdev_mbus_code_enum *code)
  {
 -   if (index = ARRAY_SIZE(adv7170_codes))
 +   if (code-pad || code-index = ARRAY_SIZE(adv7170_codes))
 return -EINVAL;

 -   *code = adv7170_codes[index];
 +   code-code = adv7170_codes[code-index];
 return 0;
  }

 @@ -323,11 +324,15 @@ static const struct v4l2_subdev_video_ops 
 adv7170_video_ops = {
 .s_routing = adv7170_s_routing,
 .s_mbus_fmt = adv7170_s_fmt,
 .g_mbus_fmt = adv7170_g_fmt,
 -   .enum_mbus_fmt  = adv7170_enum_fmt,
 +};
 +
 +static const struct v4l2_subdev_pad_ops adv7170_pad_ops = {
 +   .enum_mbus_code = adv7170_enum_mbus_code,
  };

  static const struct v4l2_subdev_ops adv7170_ops = {
 .video = adv7170_video_ops,
 +   .pad = adv7170_pad_ops,
  };

  /* --- */
 diff --git a/drivers/media/i2c/adv7175.c b/drivers/media/i2c/adv7175.c
 index d220af5..3f40304 100644
 --- a/drivers/media/i2c/adv7175.c
 +++ b/drivers/media/i2c/adv7175.c
 @@ -300,13 +300,14 @@ static int adv7175_s_routing(struct v4l2_subdev *sd,
 return 0

Re: [PATCH 6/7] v4l2: replace s_mbus_fmt by set_fmt in bridge drivers

[Lad, Prabhakar]

Hi Hans,

Thanks for the patch.

On Thu, Apr 9, 2015 at 11:21 AM, Hans Verkuil hverk...@xs4all.nl wrote:
 From: Hans Verkuil hans.verk...@cisco.com

 Replace all calls to s_mbus_fmt in bridge drivers by calls to the
 set_fmt pad op.

 Remove the old try/s_mbus_fmt video ops since they are now no longer used.

 Signed-off-by: Hans Verkuil hans.verk...@cisco.com
 Cc: Guennadi Liakhovetski g.liakhovet...@gmx.de
 Cc: Prabhakar Lad prabhakar.cse...@gmail.com
 Cc: Scott Jiang scott.jiang.li...@gmail.com
 Cc: Jonathan Corbet cor...@lwn.net
 ---
  drivers/media/pci/cx18/cx18-controls.c | 13 +++--
  drivers/media/pci/cx18/cx18-ioctl.c| 12 +++--
  drivers/media/pci/cx23885/cx23885-video.c  | 12 +++--
  drivers/media/pci/ivtv/ivtv-controls.c | 12 +++--
  drivers/media/pci/ivtv/ivtv-ioctl.c| 12 +++--
  drivers/media/pci/saa7134/saa7134-empress.c| 10 ++--
  drivers/media/platform/am437x/am437x-vpfe.c| 19 ++-
  drivers/media/platform/blackfin/bfin_capture.c |  8 +--
  drivers/media/platform/marvell-ccic/mcam-core.c|  8 +--
  drivers/media/platform/sh_vou.c| 61 
 --
  drivers/media/platform/soc_camera/atmel-isi.c  | 27 +-
  drivers/media/platform/soc_camera/mx2_camera.c | 35 +++--
  drivers/media/platform/soc_camera/mx3_camera.c | 31 ++-
  drivers/media/platform/soc_camera/omap1_camera.c   | 44 +---
  drivers/media/platform/soc_camera/pxa_camera.c | 33 ++--
  drivers/media/platform/soc_camera/rcar_vin.c   |  4 +-
  .../platform/soc_camera/sh_mobile_ceu_camera.c |  8 +--
  drivers/media/platform/soc_camera/soc_scale_crop.c | 37 +++--
  drivers/media/platform/via-camera.c|  8 +--
  drivers/media/usb/cx231xx/cx231xx-417.c| 12 +++--
  drivers/media/usb/cx231xx/cx231xx-video.c  | 23 
  drivers/media/usb/em28xx/em28xx-camera.c   | 12 +++--
  drivers/media/usb/go7007/go7007-v4l2.c | 12 +++--
  drivers/media/usb/pvrusb2/pvrusb2-hdw.c| 17 +++---
  include/media/v4l2-subdev.h|  8 ---
  25 files changed, 256 insertions(+), 222 deletions(-)

for am437x

Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com
Tested-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/7] v4l2: replace video op g_mbus_fmt by pad op get_fmt

[Lad, Prabhakar]

Hi Hans,

Thanks for the patch.

On Thu, Apr 9, 2015 at 11:21 AM, Hans Verkuil hverk...@xs4all.nl wrote:
 From: Hans Verkuil hans.verk...@cisco.com

 The g_mbus_fmt video op is a duplicate of the pad op. Replace all uses
 by the get_fmt pad op and remove the video op.

 Signed-off-by: Hans Verkuil hans.verk...@cisco.com
 Cc: Guennadi Liakhovetski g.liakhovet...@gmx.de
 Cc: Prabhakar Lad prabhakar.cse...@gmail.com
 Cc: Kamil Debski k.deb...@samsung.com
 ---
[Snip]
  drivers/media/i2c/tvp514x.c| 35 ++
  drivers/media/i2c/tvp7002.c| 28 ---
  drivers/media/platform/am437x/am437x-vpfe.c|  6 +--
  drivers/media/platform/davinci/vpfe_capture.c  | 19 

For the above,

Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2] media: i2c: ov2659: Use v4l2_of_alloc_parse_endpoint()

[Lad Prabhakar]

From: Lad, Prabhakar prabhakar.cse...@gmail.com

Instead of parsing the link-frequencies property in the driver, let
v4l2_of_alloc_parse_endpoint() do it.

Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
Acked-by: Sakari Ailus sakari.ai...@iki.fi
---
 Changes for v2:
 a: Ignoring nr_of_link_frequencies if greater then one and
just using the first one.
 b: Included Ack from Sakari
 
 v1: https://patchwork.kernel.org/patch/611/
 
 drivers/media/i2c/ov2659.c | 19 ++-
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c
index edebd11..04bb276 100644
--- a/drivers/media/i2c/ov2659.c
+++ b/drivers/media/i2c/ov2659.c
@@ -1340,8 +1340,8 @@ static struct ov2659_platform_data *
 ov2659_get_pdata(struct i2c_client *client)
 {
struct ov2659_platform_data *pdata;
+   struct v4l2_of_endpoint *bus_cfg;
struct device_node *endpoint;
-   int ret;
 
if (!IS_ENABLED(CONFIG_OF) || !client-dev.of_node)
return client-dev.platform_data;
@@ -1350,18 +1350,27 @@ ov2659_get_pdata(struct i2c_client *client)
if (!endpoint)
return NULL;
 
+   bus_cfg = v4l2_of_alloc_parse_endpoint(endpoint);
+   if (IS_ERR(bus_cfg)) {
+   pdata = NULL;
+   goto done;
+   }
+
pdata = devm_kzalloc(client-dev, sizeof(*pdata), GFP_KERNEL);
if (!pdata)
goto done;
 
-   ret = of_property_read_u64(endpoint, link-frequencies,
-  pdata-link_frequency);
-   if (ret) {
-   dev_err(client-dev, link-frequencies property not found\n);
+   if (!bus_cfg-nr_of_link_frequencies) {
+   dev_err(client-dev,
+   link-frequencies property not found or too many\n);
pdata = NULL;
+   goto done;
}
 
+   pdata-link_frequency = bus_cfg-link_frequencies[0];
+
 done:
+   v4l2_of_free_endpoint(bus_cfg);
of_node_put(endpoint);
return pdata;
 }
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] media: i2c: ov2659: Use v4l2_of_alloc_parse_endpoint()

[Lad, Prabhakar]

Hi Sakari,

Thanks for the review.

On Sat, Apr 11, 2015 at 1:48 PM, Sakari Ailus sakari.ai...@iki.fi wrote:
 Hi Prabhakar,

 On Fri, Apr 10, 2015 at 11:13:28PM +0100, Lad Prabhakar wrote:
 From: Lad, Prabhakar prabhakar.cse...@gmail.com

 Instead of parsing the link-frequencies property in the driver, let
 v4l2_of_alloc_parse_endpoint() do it.

 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
 ---
  This patch depends on https://patchwork.kernel.org/patch/6190901/

  drivers/media/i2c/ov2659.c | 19 ++-
  1 file changed, 14 insertions(+), 5 deletions(-)

 diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c
 index edebd11..c1e310b 100644
 --- a/drivers/media/i2c/ov2659.c
 +++ b/drivers/media/i2c/ov2659.c
 @@ -1340,8 +1340,8 @@ static struct ov2659_platform_data *
  ov2659_get_pdata(struct i2c_client *client)
  {
   struct ov2659_platform_data *pdata;
 + struct v4l2_of_endpoint *bus_cfg;
   struct device_node *endpoint;
 - int ret;

   if (!IS_ENABLED(CONFIG_OF) || !client-dev.of_node)
   return client-dev.platform_data;
 @@ -1350,18 +1350,27 @@ ov2659_get_pdata(struct i2c_client *client)
   if (!endpoint)
   return NULL;

 + bus_cfg = v4l2_of_alloc_parse_endpoint(endpoint);
 + if (IS_ERR(bus_cfg)) {
 + pdata = NULL;
 + goto done;
 + }
 +
   pdata = devm_kzalloc(client-dev, sizeof(*pdata), GFP_KERNEL);
   if (!pdata)
   goto done;

 - ret = of_property_read_u64(endpoint, link-frequencies,
 -pdata-link_frequency);
 - if (ret) {
 - dev_err(client-dev, link-frequencies property not found\n);
 + if (bus_cfg-nr_of_link_frequencies != 1) {

 I wonder if it should be considered a problem if the array is larger than
 one item. I would not, even if the rest of the entries wouldn't be used by
 the driver at the moment. Up to you.

OK will drop the check for more than one entries.

 Acked-by: Sakari Ailus sakari.ai...@iki.fi

Thanks for the Ack.

Cheers,
--Prabhakar Lad


 + dev_err(client-dev,
 + link-frequencies property not found or too many\n);
   pdata = NULL;
 + goto done;
   }

 + pdata-link_frequency = bus_cfg-link_frequencies[0];
 +
  done:
 + v4l2_of_free_endpoint(bus_cfg);
   of_node_put(endpoint);
   return pdata;
  }

 --
 Kind regards,

 Sakari Ailus
 e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] media: i2c: ov2659: add VIDEO_V4L2_SUBDEV_API dependency

[Lad Prabhakar]

From: Lad, Prabhakar prabhakar.cse...@gmail.com

this patch adds dependency of VIDEO_V4L2_SUBDEV_API
for VIDEO_OV2659 so that it doesn't complain for random
config builds.

Reported-by: Randy Dunlap rdun...@infradead.org
Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
---
 drivers/media/i2c/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index 6f30ea7..8b05681 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -468,7 +468,7 @@ config VIDEO_SMIAPP_PLL
 
 config VIDEO_OV2659
tristate OmniVision OV2659 sensor support
-   depends on VIDEO_V4L2  I2C
+   depends on VIDEO_V4L2  I2C  VIDEO_V4L2_SUBDEV_API
depends on MEDIA_CAMERA_SUPPORT
---help---
  This is a Video4Linux2 sensor-level driver for the OmniVision
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 4/4] smiapp: Use v4l2_of_alloc_parse_endpoint()

[Lad, Prabhakar]

Hi Sakari,

Thanks for the patch.

On Thu, Apr 9, 2015 at 10:25 PM, Sakari Ailus sakari.ai...@iki.fi wrote:
 Instead of parsing the link-frequencies property in the driver, let
 v4l2_of_alloc_parse_endpoint() do it.

 Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
 Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
 ---
  drivers/media/i2c/smiapp/smiapp-core.c |   40 
 
  1 file changed, 20 insertions(+), 20 deletions(-)

 diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
 b/drivers/media/i2c/smiapp/smiapp-core.c
 index 557f25d..4a2e8d3 100644
 --- a/drivers/media/i2c/smiapp/smiapp-core.c
 +++ b/drivers/media/i2c/smiapp/smiapp-core.c
 @@ -2975,9 +2975,9 @@ static int smiapp_resume(struct device *dev)
  static struct smiapp_platform_data *smiapp_get_pdata(struct device *dev)
  {
 struct smiapp_platform_data *pdata;
 -   struct v4l2_of_endpoint bus_cfg;
 +   struct v4l2_of_endpoint *bus_cfg;
 struct device_node *ep;
 -   uint32_t asize;
 +   int i;
 int rval;

 if (!dev-of_node)
 @@ -2987,13 +2987,17 @@ static struct smiapp_platform_data 
 *smiapp_get_pdata(struct device *dev)
 if (!ep)
 return NULL;

 +   bus_cfg = v4l2_of_alloc_parse_endpoint(ep);
 +   if (IS_ERR(bus_cfg)) {
 +   rval = PTR_ERR(bus_cfg);

this assignment  is not required.

Apart from that the patch looks good.

Reviewed-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] media: i2c: ov2659: Use v4l2_of_alloc_parse_endpoint()

[Lad Prabhakar]

From: Lad, Prabhakar prabhakar.cse...@gmail.com

Instead of parsing the link-frequencies property in the driver, let
v4l2_of_alloc_parse_endpoint() do it.

Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
---
 This patch depends on https://patchwork.kernel.org/patch/6190901/
 
 drivers/media/i2c/ov2659.c | 19 ++-
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c
index edebd11..c1e310b 100644
--- a/drivers/media/i2c/ov2659.c
+++ b/drivers/media/i2c/ov2659.c
@@ -1340,8 +1340,8 @@ static struct ov2659_platform_data *
 ov2659_get_pdata(struct i2c_client *client)
 {
struct ov2659_platform_data *pdata;
+   struct v4l2_of_endpoint *bus_cfg;
struct device_node *endpoint;
-   int ret;
 
if (!IS_ENABLED(CONFIG_OF) || !client-dev.of_node)
return client-dev.platform_data;
@@ -1350,18 +1350,27 @@ ov2659_get_pdata(struct i2c_client *client)
if (!endpoint)
return NULL;
 
+   bus_cfg = v4l2_of_alloc_parse_endpoint(endpoint);
+   if (IS_ERR(bus_cfg)) {
+   pdata = NULL;
+   goto done;
+   }
+
pdata = devm_kzalloc(client-dev, sizeof(*pdata), GFP_KERNEL);
if (!pdata)
goto done;
 
-   ret = of_property_read_u64(endpoint, link-frequencies,
-  pdata-link_frequency);
-   if (ret) {
-   dev_err(client-dev, link-frequencies property not found\n);
+   if (bus_cfg-nr_of_link_frequencies != 1) {
+   dev_err(client-dev,
+   link-frequencies property not found or too many\n);
pdata = NULL;
+   goto done;
}
 
+   pdata-link_frequency = bus_cfg-link_frequencies[0];
+
 done:
+   v4l2_of_free_endpoint(bus_cfg);
of_node_put(endpoint);
return pdata;
 }
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 3/4] v4l: of: Parse variable length properties --- link-frequencies

[Lad, Prabhakar]

Hi Sakari,

Thanks for the patch.

On Thu, Apr 9, 2015 at 10:25 PM, Sakari Ailus sakari.ai...@iki.fi wrote:
 The link-frequencies property is a variable length array of link frequencies
 in an endpoint. The array is needed by an increasing number of drivers, so
 it makes sense to add it to struct v4l2_of_endpoint.

 However, the length of the array is variable and the size of struct
 v4l2_of_endpoint is fixed since it is allocated by the caller. The options
 here are

 1. to define a fixed maximum limit of link frequencies that has to be the
 global maximum of all boards. This is seen as problematic since the maximum
 could be largish, and everyone hitting the problem would need to submit a
 patch to fix it, or

 2. parse the property in every driver. This doesn't sound appealing as two
 of the three implementations submitted to linux-media were wrong, and one of
 them was even merged before this was noticed, or

 3. change the interface so that allocating and releasing memory according to
 the size of the array is possible. This is what the patch does.

 v4l2_of_alloc_parse_endpoint() is just like v4l2_of_parse_endpoint(), but it
 will allocate the memory resources needed to store struct v4l2_of_endpoint
 and the additional arrays pointed to by this struct. A corresponding release
 function v4l2_of_free_endpoint() is provided to release the memory allocated
 by v4l2_of_alloc_parse_endpoint().

 In addition to this, the link-frequencies property is parsed as well, and
 the result is stored to struct v4l2_of_endpoint field link_frequencies.

 Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
 Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com

Tested-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Cheers,
--Prabhakar Lad

 ---
  drivers/media/v4l2-core/v4l2-of.c |   87 
 +
  include/media/v4l2-of.h   |   17 
  2 files changed, 104 insertions(+)

 diff --git a/drivers/media/v4l2-core/v4l2-of.c 
 b/drivers/media/v4l2-core/v4l2-of.c
 index 3ac6348..c52fb96 100644
 --- a/drivers/media/v4l2-core/v4l2-of.c
 +++ b/drivers/media/v4l2-core/v4l2-of.c
 @@ -14,6 +14,7 @@
  #include linux/kernel.h
  #include linux/module.h
  #include linux/of.h
 +#include linux/slab.h
  #include linux/string.h
  #include linux/types.h

 @@ -141,6 +142,10 @@ static void v4l2_of_parse_parallel_bus(const struct 
 device_node *node,
   * V4L2_MBUS_CSI2_CONTINUOUS_CLOCK flag.
   * The caller should hold a reference to @node.
   *
 + * NOTE: This function does not parse properties the size of which is
 + * variable without a low fixed limit. Please use
 + * v4l2_of_alloc_parse_endpoint() in new drivers instead.
 + *
   * Return: 0.
   */
  int v4l2_of_parse_endpoint(const struct device_node *node,
 @@ -167,6 +172,88 @@ int v4l2_of_parse_endpoint(const struct device_node 
 *node,
  }
  EXPORT_SYMBOL(v4l2_of_parse_endpoint);

 +/*
 + * v4l2_of_free_endpoint() - free the endpoint acquired by
 + * v4l2_of_alloc_parse_endpoint()
 + * @endpoint - the endpoint the resources of which are to be released
 + *
 + * It is safe to call this function with NULL argument or on an
 + * endpoint the parsing of which failed.
 + */
 +void v4l2_of_free_endpoint(struct v4l2_of_endpoint *endpoint)
 +{
 +   if (IS_ERR_OR_NULL(endpoint))
 +   return;
 +
 +   kfree(endpoint-link_frequencies);
 +   kfree(endpoint);
 +}
 +EXPORT_SYMBOL(v4l2_of_free_endpoint);
 +
 +/**
 + * v4l2_of_alloc_parse_endpoint() - parse all endpoint node properties
 + * @node: pointer to endpoint device_node
 + *
 + * All properties are optional. If none are found, we don't set any flags.
 + * This means the port has a static configuration and no properties have
 + * to be specified explicitly.
 + * If any properties that identify the bus as parallel are found and
 + * slave-mode isn't set, we set V4L2_MBUS_MASTER. Similarly, if we recognise
 + * the bus as serial CSI-2 and clock-noncontinuous isn't set, we set the
 + * V4L2_MBUS_CSI2_CONTINUOUS_CLOCK flag.
 + * The caller should hold a reference to @node.
 + *
 + * v4l2_of_alloc_parse_endpoint() has two important differences to
 + * v4l2_of_parse_endpoint():
 + *
 + * 1. It also parses variable size data and
 + *
 + * 2. The memory it has allocated to store the variable size data must
 + *be freed using v4l2_of_free_endpoint() when no longer needed.
 + *
 + * Return: Pointer to v4l2_of_endpoint if successful, on error a
 + * negative error code.
 + */
 +struct v4l2_of_endpoint *v4l2_of_alloc_parse_endpoint(
 +   const struct device_node *node)
 +{
 +   struct v4l2_of_endpoint *endpoint;
 +   int len;
 +   int rval;
 +
 +   endpoint = kzalloc(sizeof(*endpoint), GFP_KERNEL);
 +   if (!endpoint)
 +   return ERR_PTR(-ENOMEM);
 +
 +   rval = v4l2_of_parse_endpoint(node, endpoint);
 +   if (rval  0)
 +   goto out_err;
 +
 +   if (of_get_property(node, link-frequencies, len)) {
 +   endpoint

Re: [PATCH v4 2/4] v4l: of: Instead of zeroing bus_type and bus field separately, unify this

[Lad, Prabhakar]

Hi Sakari,

Thanks for the patch.

On Thu, Apr 9, 2015 at 10:25 PM, Sakari Ailus sakari.ai...@iki.fi wrote:
 Zero the entire struct starting from bus_type. As more fields are added, no
 changes will be needed in the function to reset their value explicitly.

 Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
 Acked-by: Laurent Pinchart laurent.pinch...@ideasonboard.com

Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Cheers,
--Prabhakar Lad

 ---
  drivers/media/v4l2-core/v4l2-of.c |5 +++--
  include/media/v4l2-of.h   |1 +
  2 files changed, 4 insertions(+), 2 deletions(-)

 diff --git a/drivers/media/v4l2-core/v4l2-of.c 
 b/drivers/media/v4l2-core/v4l2-of.c
 index 83143d3..3ac6348 100644
 --- a/drivers/media/v4l2-core/v4l2-of.c
 +++ b/drivers/media/v4l2-core/v4l2-of.c
 @@ -149,8 +149,9 @@ int v4l2_of_parse_endpoint(const struct device_node *node,
 int rval;

 of_graph_parse_endpoint(node, endpoint-base);
 -   endpoint-bus_type = 0;
 -   memset(endpoint-bus, 0, sizeof(endpoint-bus));
 +   /* Zero fields from bus_type to until the end */
 +   memset(endpoint-bus_type, 0, sizeof(*endpoint) -
 +  offsetof(typeof(*endpoint), bus_type));

 rval = v4l2_of_parse_csi_bus(node, endpoint);
 if (rval)
 diff --git a/include/media/v4l2-of.h b/include/media/v4l2-of.h
 index f66b92c..6c85c07 100644
 --- a/include/media/v4l2-of.h
 +++ b/include/media/v4l2-of.h
 @@ -60,6 +60,7 @@ struct v4l2_of_bus_parallel {
   */
  struct v4l2_of_endpoint {
 struct of_endpoint base;
 +   /* Fields below this line will be zeroed by v4l2_of_parse_endpoint() 
 */
 enum v4l2_mbus_type bus_type;
 union {
 struct v4l2_of_bus_parallel parallel;
 --
 1.7.10.4

 --
 To unsubscribe from this list: send the line unsubscribe linux-media in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8] media: i2c: add support for omnivision's ov2659 sensor

[Lad, Prabhakar]

On Fri, Mar 20, 2015 at 12:05 PM, Varka Bhadram varkabhad...@gmail.com wrote:
 On 03/20/2015 05:18 PM, Lad Prabhakar wrote:

 From: Benoit Parrot bpar...@ti.com

 this patch adds support for omnivision's ov2659
 sensor, the driver supports following features:
 1: Asynchronous probing
 2: DT support
 3: Media controller support

 Signed-off-by: Benoit Parrot bpar...@ti.com
 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
 Acked-by: Sakari Ailus sakari.ai...@linux.intel.com
 ---
   Changes for v8:
   --
   a. Now setting the link_frequency control in set_fmt
  callback instead of implementing g_volatile_ctrl()
  for it and setting it there.

   v7: https://patchwork.kernel.org/patch/6034651/
   v6: https://patchwork.kernel.org/patch/6012751/
   v5: https://patchwork.kernel.org/patch/6000161/
   v4: https://patchwork.kernel.org/patch/5961661/
   v3: https://patchwork.kernel.org/patch/5959401/
   v2: https://patchwork.kernel.org/patch/5859801/
   v1: https://patchwork.linuxtv.org/patch/27919/

   .../devicetree/bindings/media/i2c/ov2659.txt   |   38 +
   MAINTAINERS|   10 +
   drivers/media/i2c/Kconfig  |   11 +
   drivers/media/i2c/Makefile |1 +
   drivers/media/i2c/ov2659.c | 1528
 
   include/media/ov2659.h |   33 +
   6 files changed, 1621 insertions(+)
   create mode 100644
 Documentation/devicetree/bindings/media/i2c/ov2659.txt
   create mode 100644 drivers/media/i2c/ov2659.c
   create mode 100644 include/media/ov2659.h

 (...)

 +static struct ov2659_platform_data *
 +ov2659_get_pdata(struct i2c_client *client)
 +{
 +   struct ov2659_platform_data *pdata;
 +   struct device_node *endpoint;
 +   int ret;
 +
 +   if (!IS_ENABLED(CONFIG_OF) || !client-dev.of_node) {
 +   dev_err(client-dev, ov2659_get_pdata: DT Node
 found\n);


 ov2659_get_pdata: DT Node *not* found...?

Good catch!

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v10] media: i2c: add support for omnivision's ov2659 sensor

[Lad Prabhakar]

From: Benoit Parrot bpar...@ti.com

this patch adds support for omnivision's ov2659
sensor, the driver supports following features:
1: Asynchronous probing
2: DT support
3: Media controller support

Signed-off-by: Benoit Parrot bpar...@ti.com
Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
Acked-by: Sakari Ailus sakari.ai...@linux.intel.com
---
 Changes for v10:
 Fixed review comments pointed by Hans.

 v9: https://patchwork.kernel.org/patch/6056111/
 v8: https://patchwork.kernel.org/patch/6055981/
 v7: https://patchwork.kernel.org/patch/6034651/
 v6: https://patchwork.kernel.org/patch/6012751/
 v5: https://patchwork.kernel.org/patch/6000161/
 v4: https://patchwork.kernel.org/patch/5961661/
 v3: https://patchwork.kernel.org/patch/5959401/
 v2: https://patchwork.kernel.org/patch/5859801/
 v1: https://patchwork.linuxtv.org/patch/27919/

 .../devicetree/bindings/media/i2c/ov2659.txt   |   38 +
 MAINTAINERS|   10 +
 drivers/media/i2c/Kconfig  |   11 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov2659.c | 1509 
 include/media/ov2659.h |   34 +
 6 files changed, 1603 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2659.txt
 create mode 100644 drivers/media/i2c/ov2659.c
 create mode 100644 include/media/ov2659.h

diff --git a/Documentation/devicetree/bindings/media/i2c/ov2659.txt 
b/Documentation/devicetree/bindings/media/i2c/ov2659.txt
new file mode 100644
index 000..cabc7d8
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov2659.txt
@@ -0,0 +1,38 @@
+* OV2659 1/5-Inch 2Mp SOC Camera
+
+The Omnivision OV2659 is a 1/5-inch SOC camera, with an active array size of
+1632H x 1212V. It is programmable through a SCCB. The OV2659 sensor supports
+multiple resolutions output, such as UXGA, SVGA, 720p. It also can support
+YUV422, RGB565/555 or raw RGB output formats.
+
+Required Properties:
+- compatible: Must be ovti,ov2659
+- reg: I2C slave address
+- clocks: reference to the xvclk input clock.
+- clock-names: should be xvclk.
+- link-frequencies: target pixel clock frequency.
+
+For further reading on port node refer to
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Example:
+
+   i2c0@1c22000 {
+   ...
+   ...
+ov2659@30 {
+   compatible = ovti,ov2659;
+   reg = 0x30;
+
+   clocks = clk_ov2659 0;
+   clock-names = xvclk;
+
+   port {
+   ov2659_0: endpoint {
+   remote-endpoint = vpfe_ep;
+   link-frequencies = /bits/ 64 7000;
+   };
+   };
+   };
+   ...
+   };
diff --git a/MAINTAINERS b/MAINTAINERS
index ddc5a8c..4006cc8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8910,6 +8910,16 @@ T:   git 
git://linuxtv.org/mhadli/v4l-dvb-davinci_devices.git
 S: Maintained
 F: drivers/media/platform/am437x/
 
+OV2659 OMNIVISION SENSOR DRIVER
+M: Lad, Prabhakar prabhakar.cse...@gmail.com
+L: linux-media@vger.kernel.org
+W: http://linuxtv.org/
+Q: http://patchwork.linuxtv.org/project/linux-media/list/
+T: git git://linuxtv.org/mhadli/v4l-dvb-davinci_devices.git
+S: Maintained
+F: drivers/media/i2c/ov2659.c
+F: include/media/ov2659.h
+
 SIS 190 ETHERNET DRIVER
 M: Francois Romieu rom...@fr.zoreil.com
 L: net...@vger.kernel.org
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index da58c9b..6f30ea7 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -466,6 +466,17 @@ config VIDEO_APTINA_PLL
 config VIDEO_SMIAPP_PLL
tristate
 
+config VIDEO_OV2659
+   tristate OmniVision OV2659 sensor support
+   depends on VIDEO_V4L2  I2C
+   depends on MEDIA_CAMERA_SUPPORT
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the OmniVision
+ OV2659 camera.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ov2659.
+
 config VIDEO_OV7640
tristate OmniVision OV7640 sensor support
depends on I2C  VIDEO_V4L2
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 98589001..f165fae 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -77,3 +77,4 @@ obj-$(CONFIG_VIDEO_SMIAPP_PLL)+= smiapp-pll.o
 obj-$(CONFIG_VIDEO_AK881X) += ak881x.o
 obj-$(CONFIG_VIDEO_IR_I2C)  += ir-kbd-i2c.o
 obj-$(CONFIG_VIDEO_ML86V7667)  += ml86v7667.o
+obj-$(CONFIG_VIDEO_OV2659) += ov2659.o
diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c
new file mode 100644
index 000..edebd11
--- /dev/null
+++ b/drivers/media/i2c

Re: [PATCH v7] media: i2c: add support for omnivision's ov2659 sensor

[Lad, Prabhakar]

Hi Sakari,

Thanks for the review.

On Wed, Mar 18, 2015 at 10:13 PM, Sakari Ailus
sakari.ai...@linux.intel.com wrote:
 Hi Prabhakar,

 Lad Prabhakar wrote:
 ...

 +static int ov2659_g_volatile_ctrl(struct v4l2_ctrl *ctrl)
 +{
 +   struct ov2659 *ov2659 =
 +   container_of(ctrl-handler, struct ov2659, ctrls);
 +   struct v4l2_mbus_framefmt *fmt = ov2659-format;
 +
 +   switch (ctrl-id) {
 +   case V4L2_CID_PIXEL_RATE:
 +   if (fmt-code != MEDIA_BUS_FMT_SBGGR8_1X8)
 +   ov2659-link_frequency-val =
 +   ov2659-pdata-link_frequency / 2;
 +   else
 +   ov2659-link_frequency-val =
 +   ov2659-pdata-link_frequency;


 You should simply use v4l2_ctrl_s_ctrl_int64() in ..._set_fmt() as this
 isn't really a proper volatile control, but its value depends on the format.

Yea makes sense, will respin the patch with this change.

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6] media: i2c: add support for omnivision's ov2659 sensor

[Lad, Prabhakar]

Hi Hans,

Thanks for the review.

On Mon, Mar 16, 2015 at 9:24 AM, Hans Verkuil hverk...@xs4all.nl wrote:
 Hi Prabhakar,

 On 03/15/2015 11:34 AM, Lad Prabhakar wrote:
 From: Benoit Parrot bpar...@ti.com

 this patch adds support for omnivision's ov2659
 sensor, the driver supports following features:
 1: Asynchronous probing
 2: DT support
 3: Media controller support

 Signed-off-by: Benoit Parrot bpar...@ti.com
 Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
 Acked-by: Sakari Ailus sakari.ai...@linux.intel.com
 ---
  Changes for v6:
  a: fixed V4L2_CID_PIXEL_RATE control to use link_frequency
 instead of xvclk_frequency.
  b: Included Ack from Sakari

  v5: https://patchwork.kernel.org/patch/6000161/
  v4: https://patchwork.kernel.org/patch/5961661/
  v3: https://patchwork.kernel.org/patch/5959401/
  v2: https://patchwork.kernel.org/patch/5859801/
  v1: https://patchwork.linuxtv.org/patch/27919/

  .../devicetree/bindings/media/i2c/ov2659.txt   |   38 +
  MAINTAINERS|   10 +
  drivers/media/i2c/Kconfig  |   11 +
  drivers/media/i2c/Makefile |1 +
  drivers/media/i2c/ov2659.c | 1510 
 
  include/media/ov2659.h |   33 +
  6 files changed, 1603 insertions(+)
  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2659.txt
  create mode 100644 drivers/media/i2c/ov2659.c
  create mode 100644 include/media/ov2659.h

 diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c
 new file mode 100644
 index 000..3ae6629
 --- /dev/null
 +++ b/drivers/media/i2c/ov2659.c

 snip

 +static const struct ov2659_pixfmt ov2659_formats[] = {
 + {
 + .code = MEDIA_BUS_FMT_YUYV8_2X8,
 + .colorspace = V4L2_COLORSPACE_JPEG,
 + .format_ctrl_regs = ov2659_format_yuyv,
 + },
 + {
 + .code = MEDIA_BUS_FMT_UYVY8_2X8,
 + .colorspace = V4L2_COLORSPACE_JPEG,
 + .format_ctrl_regs = ov2659_format_uyvy,
 + },
 + {
 + .code = MEDIA_BUS_FMT_RGB565_2X8_BE,
 + .colorspace = V4L2_COLORSPACE_JPEG,
 + .format_ctrl_regs = ov2659_format_rgb565,
 + },
 + {
 + .code = MEDIA_BUS_FMT_SBGGR8_1X8,
 + .colorspace = V4L2_COLORSPACE_SMPTE170M,
 + .format_ctrl_regs = ov2659_format_bggr,
 + },
 +};

 The colorspaces defined here make no sense. Sensors should give you
 V4L2_COLORSPACE_SRGB. Certainly not COLORSPACE_JPEG (unless they encode
 to a JPEG for you) and SMPTE170M (SDTV) is unlikely as well, unless the
 documentation explicitly states that it uses that colorspace.

 Unfortunately, the product brief of this sensor does not mention the
 colorimetry information at all, nor does it give any information about
 the transfer function (aka gamma) used by the sensor. Since this sensor
 is advertised as an HDTV sensor I would guess the colorspace should either
 be SRGB or REC709, depending on the transfer function used.

Yes it needs to be V4L2_COLORSPACE_SRGB, Ill respin fixing this.

Cheers,
--Prabhakar Lad

 I see a lot of sensor drivers that wrongly use the JPEG colorspace. I'm 
 planning
 to fix them, since that is really wrong.

 Regards,

 Hans
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v6] media: i2c: add support for omnivision's ov2659 sensor

[Lad Prabhakar]

From: Benoit Parrot bpar...@ti.com

this patch adds support for omnivision's ov2659
sensor, the driver supports following features:
1: Asynchronous probing
2: DT support
3: Media controller support

Signed-off-by: Benoit Parrot bpar...@ti.com
Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
Acked-by: Sakari Ailus sakari.ai...@linux.intel.com
---
 Changes for v6:
 a: fixed V4L2_CID_PIXEL_RATE control to use link_frequency
instead of xvclk_frequency.
 b: Included Ack from Sakari
 
 v5: https://patchwork.kernel.org/patch/6000161/
 v4: https://patchwork.kernel.org/patch/5961661/
 v3: https://patchwork.kernel.org/patch/5959401/
 v2: https://patchwork.kernel.org/patch/5859801/
 v1: https://patchwork.linuxtv.org/patch/27919/

 .../devicetree/bindings/media/i2c/ov2659.txt   |   38 +
 MAINTAINERS|   10 +
 drivers/media/i2c/Kconfig  |   11 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov2659.c | 1510 
 include/media/ov2659.h |   33 +
 6 files changed, 1603 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2659.txt
 create mode 100644 drivers/media/i2c/ov2659.c
 create mode 100644 include/media/ov2659.h

diff --git a/Documentation/devicetree/bindings/media/i2c/ov2659.txt 
b/Documentation/devicetree/bindings/media/i2c/ov2659.txt
new file mode 100644
index 000..cabc7d8
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov2659.txt
@@ -0,0 +1,38 @@
+* OV2659 1/5-Inch 2Mp SOC Camera
+
+The Omnivision OV2659 is a 1/5-inch SOC camera, with an active array size of
+1632H x 1212V. It is programmable through a SCCB. The OV2659 sensor supports
+multiple resolutions output, such as UXGA, SVGA, 720p. It also can support
+YUV422, RGB565/555 or raw RGB output formats.
+
+Required Properties:
+- compatible: Must be ovti,ov2659
+- reg: I2C slave address
+- clocks: reference to the xvclk input clock.
+- clock-names: should be xvclk.
+- link-frequencies: target pixel clock frequency.
+
+For further reading on port node refer to
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Example:
+
+   i2c0@1c22000 {
+   ...
+   ...
+ov2659@30 {
+   compatible = ovti,ov2659;
+   reg = 0x30;
+
+   clocks = clk_ov2659 0;
+   clock-names = xvclk;
+
+   port {
+   ov2659_0: endpoint {
+   remote-endpoint = vpfe_ep;
+   link-frequencies = /bits/ 64 7000;
+   };
+   };
+   };
+   ...
+   };
diff --git a/MAINTAINERS b/MAINTAINERS
index ddc5a8c..4006cc8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8910,6 +8910,16 @@ T:   git 
git://linuxtv.org/mhadli/v4l-dvb-davinci_devices.git
 S: Maintained
 F: drivers/media/platform/am437x/
 
+OV2659 OMNIVISION SENSOR DRIVER
+M: Lad, Prabhakar prabhakar.cse...@gmail.com
+L: linux-media@vger.kernel.org
+W: http://linuxtv.org/
+Q: http://patchwork.linuxtv.org/project/linux-media/list/
+T: git git://linuxtv.org/mhadli/v4l-dvb-davinci_devices.git
+S: Maintained
+F: drivers/media/i2c/ov2659.c
+F: include/media/ov2659.h
+
 SIS 190 ETHERNET DRIVER
 M: Francois Romieu rom...@fr.zoreil.com
 L: net...@vger.kernel.org
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index da58c9b..6f30ea7 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -466,6 +466,17 @@ config VIDEO_APTINA_PLL
 config VIDEO_SMIAPP_PLL
tristate
 
+config VIDEO_OV2659
+   tristate OmniVision OV2659 sensor support
+   depends on VIDEO_V4L2  I2C
+   depends on MEDIA_CAMERA_SUPPORT
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the OmniVision
+ OV2659 camera.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ov2659.
+
 config VIDEO_OV7640
tristate OmniVision OV7640 sensor support
depends on I2C  VIDEO_V4L2
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 98589001..f165fae 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -77,3 +77,4 @@ obj-$(CONFIG_VIDEO_SMIAPP_PLL)+= smiapp-pll.o
 obj-$(CONFIG_VIDEO_AK881X) += ak881x.o
 obj-$(CONFIG_VIDEO_IR_I2C)  += ir-kbd-i2c.o
 obj-$(CONFIG_VIDEO_ML86V7667)  += ml86v7667.o
+obj-$(CONFIG_VIDEO_OV2659) += ov2659.o
diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c
new file mode 100644
index 000..3ae6629
--- /dev/null
+++ b/drivers/media/i2c/ov2659.c
@@ -0,0 +1,1510 @@
+/*
+ * Omnivision OV2659 CMOS Image Sensor driver
+ *
+ * Copyright (C) 2015 Texas

Re: [PATCH v5] media: i2c: add support for omnivision's ov2659 sensor

[Lad, Prabhakar]

Hi Sakari,

Thanks for the review.

On Fri, Mar 13, 2015 at 12:04 AM, Sakari Ailus sakari.ai...@iki.fi wrote:
 Hi Prabhakar,

 On Thu, Mar 12, 2015 at 11:22:36PM +, Lad Prabhakar wrote:
 ...
 +static int ov2659_probe(struct i2c_client *client,
 + const struct i2c_device_id *id)
 +{
 + const struct ov2659_platform_data *pdata = ov2659_get_pdata(client);
 + struct v4l2_subdev *sd;
 + struct ov2659 *ov2659;
 + struct clk *clk;
 + int ret;
 +
 + if (!pdata) {
 + dev_err(client-dev, platform data not specified\n);
 + return -EINVAL;
 + }
 +
 + ov2659 = devm_kzalloc(client-dev, sizeof(*ov2659), GFP_KERNEL);
 + if (!ov2659)
 + return -ENOMEM;
 +
 + ov2659-pdata = pdata;
 + ov2659-client = client;
 +
 + clk = devm_clk_get(client-dev, xvclk);
 + if (IS_ERR(clk))
 + return PTR_ERR(clk);
 +
 + ov2659-xvclk_frequency = clk_get_rate(clk);
 + if (ov2659-xvclk_frequency  600 ||
 + ov2659-xvclk_frequency  2700)
 + return -EINVAL;
 +
 + v4l2_ctrl_handler_init(ov2659-ctrls, 2);
 + v4l2_ctrl_new_std(ov2659-ctrls, ov2659_ctrl_ops,
 +   V4L2_CID_PIXEL_RATE, ov2659-xvclk_frequency,
 +   ov2659-xvclk_frequency, 1, ov2659-xvclk_frequency);

 ov2659-xvclk_frequency is the frequency of the external clock, not the
 pixel rate. If I understand correctly, you should use the value of the
 link-frequency property instead (as long as it's one pixel per clock).

this is what happens when you work late night :)

 With this fixed,

will post a v6 today.

Cheers,
--Prabhakar Lad

 Acked-by: Sakari Ailus sakari.ai...@linux.intel.com

 + v4l2_ctrl_new_std_menu_items(ov2659-ctrls, ov2659_ctrl_ops,
 +  V4L2_CID_TEST_PATTERN,
 +  ARRAY_SIZE(ov2659_test_pattern_menu) - 1,
 +  0, 0, ov2659_test_pattern_menu);
 + ov2659-sd.ctrl_handler = ov2659-ctrls;
 +
 + if (ov2659-ctrls.error) {
 + dev_err(client-dev, %s: control initialization error %d\n,
 + __func__, ov2659-ctrls.error);
 + return  ov2659-ctrls.error;
 + }
 +
 + sd = ov2659-sd;
 + client-flags |= I2C_CLIENT_SCCB;
 + v4l2_i2c_subdev_init(sd, client, ov2659_subdev_ops);
 +
 + sd-internal_ops = ov2659_subdev_internal_ops;
 + sd-flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
 +  V4L2_SUBDEV_FL_HAS_EVENTS;
 +
 +#if defined(CONFIG_MEDIA_CONTROLLER)
 + ov2659-pad.flags = MEDIA_PAD_FL_SOURCE;
 + sd-entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
 + ret = media_entity_init(sd-entity, 1, ov2659-pad, 0);
 + if (ret  0) {
 + v4l2_ctrl_handler_free(ov2659-ctrls);
 + return ret;
 + }
 +#endif
 +
 + mutex_init(ov2659-lock);
 +
 + ov2659_get_default_format(ov2659-format);
 + ov2659-frame_size = ov2659_framesizes[2];
 + ov2659-format_ctrl_regs = ov2659_formats[0].format_ctrl_regs;
 +
 + ret = ov2659_detect(sd);
 + if (ret  0)
 + goto error;
 +
 + /* Calculate the PLL register value needed */
 + ov2659_pll_calc_params(ov2659);
 +
 + ret = v4l2_async_register_subdev(ov2659-sd);
 + if (ret)
 + goto error;
 +
 + dev_info(client-dev, %s sensor driver registered !!\n, sd-name);
 +
 + return 0;
 +
 +error:
 + v4l2_ctrl_handler_free(ov2659-ctrls);
 +#if defined(CONFIG_MEDIA_CONTROLLER)
 + media_entity_cleanup(sd-entity);
 +#endif
 + mutex_destroy(ov2659-lock);
 + return ret;
 +}

 --
 Kind regards,

 Sakari Ailus
 e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] v4l: mt9v032: Add OF support

[Lad, Prabhakar]

Hi Laurent,

Thanks for the patch.

On Fri, Mar 13, 2015 at 12:03 AM, Laurent Pinchart
laurent.pinch...@ideasonboard.com wrote:
 Parse DT properties into a platform data structure when a DT node is
 available.

 Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com

 ---

[snip]
 +static struct mt9v032_platform_data *
 +mt9v032_get_pdata(struct i2c_client *client)
 +{
 +   struct mt9v032_platform_data *pdata;
 +   struct v4l2_of_endpoint endpoint;
 +   struct device_node *np;
 +   struct property *prop;
 +
 +   if (!IS_ENABLED(CONFIG_OF) || !client-dev.of_node)
 +   return client-dev.platform_data;
 +
 +   np = v4l2_of_get_next_endpoint(client-dev.of_node, NULL);
 +   if (!np)
 +   return NULL;
 +
 +   if (v4l2_of_parse_endpoint(np, endpoint)  0)
 +   goto done;
 +
with the above two statements it seems its based on older version of kernel.

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v5] media: i2c: add support for omnivision's ov2659 sensor

[Lad Prabhakar]

From: Benoit Parrot bpar...@ti.com

this patch adds support for omnivision's ov2659
sensor, the driver supports following features:
1: Asynchronous probing
2: DT support
3: Media controller support

Signed-off-by: Benoit Parrot bpar...@ti.com
Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com
---
 Changes for v5:
 a: Implemented V4L2_CID_PIXEL_RATE control.
 b: Fixed review comments pointed by Sakari.

 v4: https://patchwork.kernel.org/patch/5961661/
 v3: https://patchwork.kernel.org/patch/5959401/
 v2: https://patchwork.kernel.org/patch/5859801/
 v1: https://patchwork.linuxtv.org/patch/27919/
 
 .../devicetree/bindings/media/i2c/ov2659.txt   |   38 +
 MAINTAINERS|   10 +
 drivers/media/i2c/Kconfig  |   11 +
 drivers/media/i2c/Makefile |1 +
 drivers/media/i2c/ov2659.c | 1510 
 include/media/ov2659.h |   33 +
 6 files changed, 1603 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2659.txt
 create mode 100644 drivers/media/i2c/ov2659.c
 create mode 100644 include/media/ov2659.h

diff --git a/Documentation/devicetree/bindings/media/i2c/ov2659.txt 
b/Documentation/devicetree/bindings/media/i2c/ov2659.txt
new file mode 100644
index 000..cabc7d8
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/i2c/ov2659.txt
@@ -0,0 +1,38 @@
+* OV2659 1/5-Inch 2Mp SOC Camera
+
+The Omnivision OV2659 is a 1/5-inch SOC camera, with an active array size of
+1632H x 1212V. It is programmable through a SCCB. The OV2659 sensor supports
+multiple resolutions output, such as UXGA, SVGA, 720p. It also can support
+YUV422, RGB565/555 or raw RGB output formats.
+
+Required Properties:
+- compatible: Must be ovti,ov2659
+- reg: I2C slave address
+- clocks: reference to the xvclk input clock.
+- clock-names: should be xvclk.
+- link-frequencies: target pixel clock frequency.
+
+For further reading on port node refer to
+Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Example:
+
+   i2c0@1c22000 {
+   ...
+   ...
+ov2659@30 {
+   compatible = ovti,ov2659;
+   reg = 0x30;
+
+   clocks = clk_ov2659 0;
+   clock-names = xvclk;
+
+   port {
+   ov2659_0: endpoint {
+   remote-endpoint = vpfe_ep;
+   link-frequencies = /bits/ 64 7000;
+   };
+   };
+   };
+   ...
+   };
diff --git a/MAINTAINERS b/MAINTAINERS
index ddc5a8c..4006cc8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8910,6 +8910,16 @@ T:   git 
git://linuxtv.org/mhadli/v4l-dvb-davinci_devices.git
 S: Maintained
 F: drivers/media/platform/am437x/
 
+OV2659 OMNIVISION SENSOR DRIVER
+M: Lad, Prabhakar prabhakar.cse...@gmail.com
+L: linux-media@vger.kernel.org
+W: http://linuxtv.org/
+Q: http://patchwork.linuxtv.org/project/linux-media/list/
+T: git git://linuxtv.org/mhadli/v4l-dvb-davinci_devices.git
+S: Maintained
+F: drivers/media/i2c/ov2659.c
+F: include/media/ov2659.h
+
 SIS 190 ETHERNET DRIVER
 M: Francois Romieu rom...@fr.zoreil.com
 L: net...@vger.kernel.org
diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
index da58c9b..6f30ea7 100644
--- a/drivers/media/i2c/Kconfig
+++ b/drivers/media/i2c/Kconfig
@@ -466,6 +466,17 @@ config VIDEO_APTINA_PLL
 config VIDEO_SMIAPP_PLL
tristate
 
+config VIDEO_OV2659
+   tristate OmniVision OV2659 sensor support
+   depends on VIDEO_V4L2  I2C
+   depends on MEDIA_CAMERA_SUPPORT
+   ---help---
+ This is a Video4Linux2 sensor-level driver for the OmniVision
+ OV2659 camera.
+
+ To compile this driver as a module, choose M here: the
+ module will be called ov2659.
+
 config VIDEO_OV7640
tristate OmniVision OV7640 sensor support
depends on I2C  VIDEO_V4L2
diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
index 98589001..f165fae 100644
--- a/drivers/media/i2c/Makefile
+++ b/drivers/media/i2c/Makefile
@@ -77,3 +77,4 @@ obj-$(CONFIG_VIDEO_SMIAPP_PLL)+= smiapp-pll.o
 obj-$(CONFIG_VIDEO_AK881X) += ak881x.o
 obj-$(CONFIG_VIDEO_IR_I2C)  += ir-kbd-i2c.o
 obj-$(CONFIG_VIDEO_ML86V7667)  += ml86v7667.o
+obj-$(CONFIG_VIDEO_OV2659) += ov2659.o
diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c
new file mode 100644
index 000..1cdbb63
--- /dev/null
+++ b/drivers/media/i2c/ov2659.c
@@ -0,0 +1,1510 @@
+/*
+ * Omnivision OV2659 CMOS Image Sensor driver
+ *
+ * Copyright (C) 2015 Texas Instruments, Inc.
+ *
+ * Benoit Parrot bpar...@ti.com
+ * Lad, Prabhakar prabhakar.cse...@gmail.com
+ *
+ * This program is free

Re: [PATCH] v4l: mt9p031: Convert to the gpiod API

[Lad, Prabhakar]

Hi Laurent,

On Wed, Mar 11, 2015 at 6:29 PM, Laurent Pinchart
laurent.pinch...@ideasonboard.com wrote:
 Hi Prabhakar,

 On Tuesday 10 March 2015 18:29:22 Lad, Prabhakar wrote:
 On Sun, Mar 8, 2015 at 1:40 PM, Laurent Pinchart wrote:
  This simplifies platform data and DT integration.
 
  Signed-off-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
  ---
 
   drivers/media/i2c/mt9p031.c | 31 +++
   include/media/mt9p031.h |  2 --
   2 files changed, 11 insertions(+), 22 deletions(-)
 
  diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
  index 89ae2b4..1757ef6 100644
  --- a/drivers/media/i2c/mt9p031.c
  +++ b/drivers/media/i2c/mt9p031.c

 [snip]

  @@ -332,8 +331,8 @@ static int mt9p031_power_on(struct mt9p031 *mt9p031)
  }
 
  /* Now RESET_BAR must be high */
  -   if (gpio_is_valid(mt9p031-reset)) {
  -   gpio_set_value(mt9p031-reset, 1);
  +   if (mt9p031-reset) {
  +   gpiod_set_value(mt9p031-reset, 0);
  usleep_range(1000, 2000);
  }

 As per the data sheet reset needs to be low initially and then high,
 you just reversed it.

 The gpiod_ API will take the GPIO active-low flag into account, so the above
 gpiod_set_value(0) call will set the GPIO level to 1.

Ahh I missed it.

Reviewed-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] media: i2c: add support for omnivision's ov2659 sensor

[Lad, Prabhakar]

Hi Laurent,

Thanks for the review.

On Wed, Mar 11, 2015 at 6:22 PM, Laurent Pinchart
laurent.pinch...@ideasonboard.com wrote:
 On Tuesday 10 March 2015 03:35:57 Sakari Ailus wrote:
 On Sun, Mar 08, 2015 at 11:33:27AM +, Lad Prabhakar wrote:
 ...

  +static struct ov2659_platform_data *
  +ov2659_get_pdata(struct i2c_client *client)
  +{
  +   struct ov2659_platform_data *pdata;
  +   struct device_node *endpoint;
  +   int ret;
  +
  +   if (!IS_ENABLED(CONFIG_OF) || !client-dev.of_node) {
  +   dev_err(client-dev, ov2659_get_pdata: DT Node found\n);
  +   return client-dev.platform_data;
  +   }
  +
  +   endpoint = of_graph_get_next_endpoint(client-dev.of_node, NULL);
  +   if (!endpoint)
  +   return NULL;
  +
  +   pdata = devm_kzalloc(client-dev, sizeof(*pdata), GFP_KERNEL);
  +   if (!pdata)
  +   goto done;
  +
  +   ret = of_property_read_u32(endpoint, link-frequencies,
  +  pdata-link_frequency);

 This is actually documented as being a 64-bit array.

 The main purpose of link-frequencies is to restrict the link frequencies
 acceptable in the system due to EMI requirements. One link frequency should be
 selected in the array based on the desired format and frame rate. This is
 usually done by exposing the frequency to userspace through a writable
 V4L2_CID_LINK_FREQ control, and exposing the resulting pixel rate as a read-
 only V4L2_CID_PIXEL_RATE control.

 V4L2_CID_PIXEL_RATE is mandatory to use the sensor with several drivers
 (including omap3isp and omap4iss), so it should really be implemented.

Even if the sensor supports only one frequency the control needs to be
implemented ?

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/3] smiapp: Read link-frequencies property from the endpoint node

[Lad, Prabhakar]

On Wed, Mar 11, 2015 at 10:25 PM, Lad, Prabhakar
prabhakar.cse...@gmail.com wrote:
 Hi Sakari,

 Thanks for the patch.

 On Tue, Mar 10, 2015 at 1:18 AM, Sakari Ailus sakari.ai...@iki.fi wrote:
 The documentation stated that the link-frequencies property belongs to the
 endpoint node, not to the device's of_node. Fix this.

 There are no DT board descriptions using the driver yet, so a fix in the
 driver is sufficient.

 Signed-off-by: Sakari Ailus sakari.ai...@iki.fi

 Tested-by: Lad, Prabhakar prabhakar.cse...@gmail.com


I meant :
Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/3] smiapp: Clean up smiapp_get_pdata()

[Lad, Prabhakar]

On Wed, Mar 11, 2015 at 10:24 PM, Lad, Prabhakar
prabhakar.cse...@gmail.com wrote:
 Hi Sakari,

 Thanks for the patch.

 On Tue, Mar 10, 2015 at 1:18 AM, Sakari Ailus sakari.ai...@iki.fi wrote:
 Don't set rval when it's not used (the function returns a pointer to struct
 smiapp_platform_data).

 Signed-off-by: Sakari Ailus sakari.ai...@iki.fi

 Tested-by: Lad, Prabhakar prabhakar.cse...@gmail.com

I meant :
Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Cheers,
--Prabhakar Lad
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/3] v4l: of: Add link-frequencies array to struct v4l2_of_endpoint

[Lad, Prabhakar]

Hi Sakari,

Thanks for the patch.

On Tue, Mar 10, 2015 at 1:18 AM, Sakari Ailus sakari.ai...@iki.fi wrote:
 Parse and read the link-frequencies property in v4l2_of_parse_endpoint().
 The property is an u64 array of undefined length, thus the memory allocation
 may fail, leading

 - v4l2_of_parse_endpoint() to return an error in such a case (as well as
   when failing to parse the property) and
 - to requiring releasing the memory reserved for the array
   (v4l2_of_release_endpoint()).

 If a driver does not need to access properties that require memory
 allocation (such as link-frequencies), it may choose to call
 v4l2_of_release_endpoint() right after calling v4l2_of_parse_endpoint().

 Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
 ---
  drivers/media/i2c/adv7604.c|1 +
  drivers/media/i2c/s5c73m3/s5c73m3-core.c   |1 +
  drivers/media/i2c/s5k5baf.c|1 +
  drivers/media/i2c/smiapp/smiapp-core.c |   32 
  drivers/media/i2c/tvp514x.c|1 +
  drivers/media/i2c/tvp7002.c|1 +
  drivers/media/platform/am437x/am437x-vpfe.c|1 +
  drivers/media/platform/exynos4-is/media-dev.c  |1 +
  drivers/media/platform/exynos4-is/mipi-csis.c  |1 +
  drivers/media/platform/soc_camera/atmel-isi.c  |1 +
  drivers/media/platform/soc_camera/pxa_camera.c |1 +
  drivers/media/platform/soc_camera/rcar_vin.c   |1 +
  drivers/media/v4l2-core/v4l2-of.c  |   47 
 +++-
  include/media/v4l2-of.h|9 +
  14 files changed, 81 insertions(+), 18 deletions(-)

[snip]
 diff --git a/drivers/media/v4l2-core/v4l2-of.c 
 b/drivers/media/v4l2-core/v4l2-of.c
 index b4ed9a9..e24610c 100644
 --- a/drivers/media/v4l2-core/v4l2-of.c
 +++ b/drivers/media/v4l2-core/v4l2-of.c
 @@ -14,6 +14,7 @@
  #include linux/kernel.h
  #include linux/module.h
  #include linux/of.h
 +#include linux/slab.h
  #include linux/string.h
  #include linux/types.h

 @@ -109,6 +110,26 @@ static void v4l2_of_parse_parallel_bus(const struct 
 device_node *node,
  }

  /**
 + * v4l2_of_release_endpoint() - release resources acquired by
 + * v4l2_of_parse_endpoint()
 + * @endpoint - the endpoint the resources of which are to be released
 + *
 + * It is safe to call this function on an endpoint which is not parsed or
 + * and endpoint the parsing of which failed. However in the former case the
 + * argument must point to a struct the memory of which has been set to zero.
 + *
 + * Values in the struct v4l2_of_endpoint that are not connected to resources
 + * acquired by v4l2_of_parse_endpoint() are guaranteed to remain untouched.
 + */
 +void v4l2_of_release_endpoint(struct v4l2_of_endpoint *endpoint)
 +{
 +   kfree(endpoint-link_frequencies);
 +   endpoint-link_frequencies = NULL;
 +   endpoint-nr_of_link_frequencies = 0;
 +}
 +EXPORT_SYMBOL(v4l2_of_parse_endpoint);
 +
 +/**
   * v4l2_of_parse_endpoint() - parse all endpoint node properties
   * @node: pointer to endpoint device_node
   * @endpoint: pointer to the V4L2 OF endpoint data structure
 @@ -122,15 +143,39 @@ static void v4l2_of_parse_parallel_bus(const struct 
 device_node *node,
   * V4L2_MBUS_CSI2_CONTINUOUS_CLOCK flag.
   * The caller should hold a reference to @node.
   *
 + * An endpoint parsed using v4l2_of_parse_endpoint() must be released using
 + * v4l2_of_release_endpoint().
 + *
   * Return: 0.
   */
  int v4l2_of_parse_endpoint(const struct device_node *node,
struct v4l2_of_endpoint *endpoint)
  {
 +   int len;
 +
 of_graph_parse_endpoint(node, endpoint-base);
 endpoint-bus_type = 0;
 memset(endpoint-bus, 0, sizeof(endpoint-bus));

endpoint-link_frequencies = NULL; required here.

Apart from that patch looks good.

Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com
Tested-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Cheers,
--Prabhakar Lad

 +   if (of_get_property(node, link-frequencies, len)) {
 +   int rval;
 +
 +   endpoint-link_frequencies = kmalloc(len, GFP_KERNEL);
 +   if (!endpoint-link_frequencies)
 +   return -ENOMEM;
 +
 +   endpoint-nr_of_link_frequencies =
 +   len / sizeof(*endpoint-link_frequencies);
 +
 +   rval = of_property_read_u64_array(
 +   node, link-frequencies, endpoint-link_frequencies,
 +   endpoint-nr_of_link_frequencies);
 +   if (rval  0) {
 +   v4l2_of_release_endpoint(endpoint);
 +   return rval;
 +   }
 +   }
 +
 v4l2_of_parse_csi_bus(node, endpoint);
 /*
  * Parse the parallel video bus properties only if none
 @@ -141,4 +186,4 @@ int v4l2_of_parse_endpoint(const struct device_node *node,

 return 0;
  }
 -EXPORT_SYMBOL(v4l2_of_parse_endpoint

Re: [PATCH 2/3] smiapp: Read link-frequencies property from the endpoint node

[Lad, Prabhakar]

Hi Sakari,

Thanks for the patch.

On Tue, Mar 10, 2015 at 1:18 AM, Sakari Ailus sakari.ai...@iki.fi wrote:
 The documentation stated that the link-frequencies property belongs to the
 endpoint node, not to the device's of_node. Fix this.

 There are no DT board descriptions using the driver yet, so a fix in the
 driver is sufficient.

 Signed-off-by: Sakari Ailus sakari.ai...@iki.fi

Tested-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Cheers,
--Prabhakar Lad

 ---
  drivers/media/i2c/smiapp/smiapp-core.c |5 ++---
  1 file changed, 2 insertions(+), 3 deletions(-)

 diff --git a/drivers/media/i2c/smiapp/smiapp-core.c 
 b/drivers/media/i2c/smiapp/smiapp-core.c
 index 565a00c..ecae76b 100644
 --- a/drivers/media/i2c/smiapp/smiapp-core.c
 +++ b/drivers/media/i2c/smiapp/smiapp-core.c
 @@ -3022,8 +3022,7 @@ static struct smiapp_platform_data 
 *smiapp_get_pdata(struct device *dev)
 dev_dbg(dev, reset %d, nvm %d, clk %d, csi %d\n, pdata-xshutdown,
 pdata-nvm_size, pdata-ext_clk, pdata-csi_signalling_mode);

 -   rval = of_get_property(
 -   dev-of_node, link-frequencies, asize) ? 0 : -ENOENT;
 +   rval = of_get_property(ep, link-frequencies, asize) ? 0 : -ENOENT;
 if (rval) {
 dev_warn(dev, can't get link-frequencies array size\n);
 goto out_err;
 @@ -3037,7 +3036,7 @@ static struct smiapp_platform_data 
 *smiapp_get_pdata(struct device *dev)

 asize /= sizeof(*pdata-op_sys_clock);
 rval = of_property_read_u64_array(
 -   dev-of_node, link-frequencies, pdata-op_sys_clock, asize);
 +   ep, link-frequencies, pdata-op_sys_clock, asize);
 if (rval) {
 dev_warn(dev, can't get link-frequencies\n);
 goto out_err;
 --
 1.7.10.4

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html