Re: [PATCH v2 5/5] v4l: Renesas R-Car VSP1 driver

2013-07-31 Thread Laurent Pinchart
Hi Sakari,

On Thursday 25 July 2013 16:43:28 Sakari Ailus wrote:
 On Thu, Jul 25, 2013 at 01:46:54PM +0200, Laurent Pinchart wrote:
   On Wed, Jul 17, 2013 at 04:54:42PM +0200, Laurent Pinchart wrote:
   ...
   
+static void vsp1_device_init(struct vsp1_device *vsp1)
+{
+   unsigned int i;
+   u32 status;
+
+   /* Reset any channel that might be running. */
+   status = vsp1_read(vsp1, VI6_STATUS);
+
+   for (i = 0; i  VPS1_MAX_WPF; ++i) {
+   unsigned int timeout;
+
+   if (!(status  VI6_STATUS_SYS_ACT(i)))
+   continue;
+
+   vsp1_write(vsp1, VI6_SRESET, VI6_SRESET_SRTS(i));
+   for (timeout = 10; timeout  0; --timeout) {
+   status = vsp1_read(vsp1, VI6_STATUS);
+   if (!(status  VI6_STATUS_SYS_ACT(i)))
+   break;
+
+   usleep_range(1000, 2000);
+   }
+
+   if (timeout)
+   dev_err(vsp1-dev, failed to reset wpf.%u\n, 
i);
   
   Have you seen this happening in practice? Do you expect the device to
   function if resetting it fails?
  
  I've seen this happening during development when I had messed up register
  values, but not otherwise. I don't expect the deviec to still function if
  resetting the WPF fails, but I need to make sure that the busy loop exits.
 
 Shouldn't you also return an error in this case? The function currently
 returns void.

I will fix that.

 ...
 
+   /* Follow links downstream for each input and make sure the 
graph
+* contains no loop and that all branches end at the output WPF.
+*/
   
   I wonder if checking for loops should be done already in pipeline
   validation done by the framework. That's fine to do later on IMHO, too.
  
  It would have to be performed by the core, as the callbacks are local to
  links. That's feasible (but should be optional, as some devices might
  support circular graphs), feel free to submit a patch :-)
 
 As a matter of fact I think I will. I'd like you to test it though since I
 have no hardware with such media graph. :-)

Sure :-)

 But please don't expect to see that in time for your driver to get in. Let's
 think about that later on.

OK.

-- 
Regards,

Laurent Pinchart

--
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 5/5] v4l: Renesas R-Car VSP1 driver

2013-07-25 Thread Laurent Pinchart
Hi Sakari,

On Thursday 25 July 2013 01:48:58 Sakari Ailus wrote:
 Hi Laurent,
 
 What a nice driver! A few minor comments below:

Thank you :-)

 On Wed, Jul 17, 2013 at 04:54:42PM +0200, Laurent Pinchart wrote:
 ...
 
  +static void vsp1_device_init(struct vsp1_device *vsp1)
  +{
  +   unsigned int i;
  +   u32 status;
  +
  +   /* Reset any channel that might be running. */
  +   status = vsp1_read(vsp1, VI6_STATUS);
  +
  +   for (i = 0; i  VPS1_MAX_WPF; ++i) {
  +   unsigned int timeout;
  +
  +   if (!(status  VI6_STATUS_SYS_ACT(i)))
  +   continue;
  +
  +   vsp1_write(vsp1, VI6_SRESET, VI6_SRESET_SRTS(i));
  +   for (timeout = 10; timeout  0; --timeout) {
  +   status = vsp1_read(vsp1, VI6_STATUS);
  +   if (!(status  VI6_STATUS_SYS_ACT(i)))
  +   break;
  +
  +   usleep_range(1000, 2000);
  +   }
  +
  +   if (timeout)
  +   dev_err(vsp1-dev, failed to reset wpf.%u\n, i);
 
 Have you seen this happening in practice? Do you expect the device to
 function if resetting it fails?

I've seen this happening during development when I had messed up register 
values, but not otherwise. I don't expect the deviec to still function if 
resetting the WPF fails, but I need to make sure that the busy loop exits.

  +   }
  +
  +   vsp1_write(vsp1, VI6_CLK_DCSWT, (8  VI6_CLK_DCSWT_CSTPW_SHIFT) |
  +  (8  VI6_CLK_DCSWT_CSTRW_SHIFT));
  +
  +   for (i = 0; i  VPS1_MAX_RPF; ++i)
  +   vsp1_write(vsp1, VI6_DPR_RPF_ROUTE(i), VI6_DPR_NODE_UNUSED);
  +
  +   for (i = 0; i  VPS1_MAX_UDS; ++i)
  +   vsp1_write(vsp1, VI6_DPR_UDS_ROUTE(i), VI6_DPR_NODE_UNUSED);
  +
  +   vsp1_write(vsp1, VI6_DPR_SRU_ROUTE, VI6_DPR_NODE_UNUSED);
  +   vsp1_write(vsp1, VI6_DPR_LUT_ROUTE, VI6_DPR_NODE_UNUSED);
  +   vsp1_write(vsp1, VI6_DPR_CLU_ROUTE, VI6_DPR_NODE_UNUSED);
  +   vsp1_write(vsp1, VI6_DPR_HST_ROUTE, VI6_DPR_NODE_UNUSED);
  +   vsp1_write(vsp1, VI6_DPR_HSI_ROUTE, VI6_DPR_NODE_UNUSED);
  +   vsp1_write(vsp1, VI6_DPR_BRU_ROUTE, VI6_DPR_NODE_UNUSED);
  +
  +   vsp1_write(vsp1, VI6_DPR_HGO_SMPPT, (7  VI6_DPR_SMPPT_TGW_SHIFT) |
  +  (VI6_DPR_NODE_UNUSED  VI6_DPR_SMPPT_PT_SHIFT));
  +   vsp1_write(vsp1, VI6_DPR_HGT_SMPPT, (7  VI6_DPR_SMPPT_TGW_SHIFT) |
  +  (VI6_DPR_NODE_UNUSED  VI6_DPR_SMPPT_PT_SHIFT));
  +}
 
 ...
 
  +int vsp1_entity_init(struct vsp1_device *vsp1, struct vsp1_entity
  *entity,
  +unsigned int num_pads)
  +{
  +   static const struct {
  +   unsigned int id;
  +   unsigned int reg;
  +   } routes[] = {
  +   { VI6_DPR_NODE_LIF, 0 },
  +   { VI6_DPR_NODE_RPF(0), VI6_DPR_RPF_ROUTE(0) },
  +   { VI6_DPR_NODE_RPF(1), VI6_DPR_RPF_ROUTE(1) },
  +   { VI6_DPR_NODE_RPF(2), VI6_DPR_RPF_ROUTE(2) },
  +   { VI6_DPR_NODE_RPF(3), VI6_DPR_RPF_ROUTE(3) },
  +   { VI6_DPR_NODE_RPF(4), VI6_DPR_RPF_ROUTE(4) },
  +   { VI6_DPR_NODE_UDS(0), VI6_DPR_UDS_ROUTE(0) },
  +   { VI6_DPR_NODE_UDS(1), VI6_DPR_UDS_ROUTE(1) },
  +   { VI6_DPR_NODE_UDS(2), VI6_DPR_UDS_ROUTE(2) },
  +   { VI6_DPR_NODE_WPF(0), 0 },
  +   { VI6_DPR_NODE_WPF(1), 0 },
  +   { VI6_DPR_NODE_WPF(2), 0 },
  +   { VI6_DPR_NODE_WPF(3), 0 },
  +   };
  +
  +   unsigned int i;
  +   int ret;
  +
  +   for (i = 0; i  ARRAY_SIZE(routes); ++i) {
  +   if (routes[i].id == entity-id) {
  +   entity-route = routes[i].reg;
  +   break;
  +   }
  +   }
  +
  +   if (i == ARRAY_SIZE(routes))
  +   return -EINVAL;
  +
  +   entity-vsp1 = vsp1;
  +   entity-source_pad = num_pads - 1;
  +
  +   /* Allocate formats and pads. */
  +   entity-formats = devm_kzalloc(vsp1-dev,
  +  num_pads * sizeof(*entity-formats),
  +  GFP_KERNEL);
  +   if (entity-formats == NULL)
  +   return -ENOMEM;
  +
  +   entity-pads = devm_kzalloc(vsp1-dev, num_pads * sizeof(*entity-
pads),
  +   GFP_KERNEL);
  +   if (entity-pads == NULL)
  +   return -ENOMEM;
  +
  +   /* Initialize pads. */
  +   for (i = 0; i  num_pads - 1; ++i)
  +   entity-pads[i].flags = MEDIA_PAD_FL_SINK;
  +
  +   entity-pads[num_pads - 1].flags = MEDIA_PAD_FL_SOURCE;
  +
  +   /* Initialize the media entity. */
  +   ret = media_entity_init(entity-subdev.entity, num_pads,
  +   entity-pads, 0);
 
 How about return media_entity_init(...) instead?

Will do.

  +   if (ret  0)
  +   return ret;
  +
  +   return 0;
  +}
 
 ...
 
  +static int lif_enum_mbus_code(struct v4l2_subdev *subdev,
  + struct v4l2_subdev_fh *fh,
  + struct v4l2_subdev_mbus_code_enum *code)
  +{
  +   static const unsigned int codes[] = {
  +   V4L2_MBUS_FMT_ARGB_1X32,
  +  

Re: [PATCH v2 5/5] v4l: Renesas R-Car VSP1 driver

2013-07-25 Thread Sakari Ailus
Hi Laurent,

On Thu, Jul 25, 2013 at 01:46:54PM +0200, Laurent Pinchart wrote:
  On Wed, Jul 17, 2013 at 04:54:42PM +0200, Laurent Pinchart wrote:
  ...
  
   +static void vsp1_device_init(struct vsp1_device *vsp1)
   +{
   + unsigned int i;
   + u32 status;
   +
   + /* Reset any channel that might be running. */
   + status = vsp1_read(vsp1, VI6_STATUS);
   +
   + for (i = 0; i  VPS1_MAX_WPF; ++i) {
   + unsigned int timeout;
   +
   + if (!(status  VI6_STATUS_SYS_ACT(i)))
   + continue;
   +
   + vsp1_write(vsp1, VI6_SRESET, VI6_SRESET_SRTS(i));
   + for (timeout = 10; timeout  0; --timeout) {
   + status = vsp1_read(vsp1, VI6_STATUS);
   + if (!(status  VI6_STATUS_SYS_ACT(i)))
   + break;
   +
   + usleep_range(1000, 2000);
   + }
   +
   + if (timeout)
   + dev_err(vsp1-dev, failed to reset wpf.%u\n, i);
  
  Have you seen this happening in practice? Do you expect the device to
  function if resetting it fails?
 
 I've seen this happening during development when I had messed up register 
 values, but not otherwise. I don't expect the deviec to still function if 
 resetting the WPF fails, but I need to make sure that the busy loop exits.

Shouldn't you also return an error in this case? The function currently
returns void.

...

   + /* Follow links downstream for each input and make sure the graph
   +  * contains no loop and that all branches end at the output WPF.
   +  */
  
  I wonder if checking for loops should be done already in pipeline validation
  done by the framework. That's fine to do later on IMHO, too.
 
 It would have to be performed by the core, as the callbacks are local to 
 links. That's feasible (but should be optional, as some devices might support 
 circular graphs), feel free to submit a patch :-)

As a matter of fact I think I will. I'd like you to test it though since I
have no hardware with such media graph. :-)

But please don't expect to see that in time for your driver to get in. Let's
think about that later on.

-- 
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 5/5] v4l: Renesas R-Car VSP1 driver

2013-07-24 Thread Katsuya MATSUBARA

Hi Laurent,

Thank you for your great work for VSP1.
Some comments below.

From: Laurent Pinchart laurent.pinchart+rene...@ideasonboard.com
Date: Wed, 17 Jul 2013 16:54:42 +0200

(snip)
 +++ b/drivers/media/platform/vsp1/vsp1_drv.c
 +/*
 + * vsp1_drv.c  --  R-Car VSP1 Driver
 + *
 + * Copyright (C) 2013 Renesas Corporation
 + *
 + * Contact: Laurent Pinchart (laurent.pinch...@ideasonboard.com)
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License as published by
 + * the Free Software Foundation; either version 2 of the License, or
 + * (at your option) any later version.
 + */
 +
 +#include linux/clk.h
 +#include linux/delay.h
 +#include linux/device.h
 +#include linux/interrupt.h
 +#include linux/module.h
 +#include linux/platform_device.h
 +#include linux/videodev2.h
 +
 +#include vsp1.h
 +#include vsp1_lif.h
 +#include vsp1_rwpf.h
 +#include vsp1_uds.h
 +
 +/* 
 -
 + * Interrupt Handling
 + */
 +
 +static irqreturn_t vsp1_irq_handler(int irq, void *data)
 +{
 + struct vsp1_device *vsp1 = data;
 + irqreturn_t ret = IRQ_NONE;
 + unsigned int i;
 +
 + for (i = 0; i  VPS1_MAX_WPF; ++i) {
 + struct vsp1_rwpf *wpf = vsp1-wpf[i];
 + struct vsp1_pipeline *pipe;
 + u32 status;
 +
 + if (wpf == NULL)
 + continue;
 +
 + pipe = to_vsp1_pipeline(wpf-entity.subdev.entity);
 + status = vsp1_read(vsp1, VI6_WPF_IRQ_STA(i));
 + vsp1_write(vsp1, VI6_WPF_IRQ_STA(i), ~status);

The value set into the VI6_WPF_IRQ_STA register should be masked
with VI6_WFP_IRQ_STA_FRE since unused (upper) bits in the register
must be written with 0.

(snip)
 +static int vsp1_create_links(struct vsp1_device *vsp1, struct vsp1_entity 
 *sink)
 +{
 + struct media_entity *entity = sink-subdev.entity;
 + struct vsp1_entity *source;
 + unsigned int pad;
 + int ret;
 +
 + list_for_each_entry(source, vsp1-entities, list_dev) {
 + u32 flags;
 +
 + if (source-type == sink-type)
 + continue;
 +
 + if (source-type == VSP1_ENTITY_LIF ||
 + source-type == VSP1_ENTITY_WPF)
 + continue;
 +
 + flags = source-type == VSP1_ENTITY_RPF 
 + sink-type == VSP1_ENTITY_WPF 
 + source-index == sink-index
 +   ? MEDIA_LNK_FL_ENABLED : 0;
 +
 + for (pad = 0; pad  entity-num_pads; ++pad) {
 + if (!(entity-pads[pad].flags  MEDIA_PAD_FL_SINK))
 + continue;
 +
 + ret = media_entity_create_link(source-subdev.entity,
 +source-source_pad,
 +entity, pad, flags);
 + if (ret  0)
 + return ret;

This initialization enables some of links as the initial status.
I think link_setup() for each linked entity should be invoked here
to set up the sink value in the vsp_entity structure.

(snip)
 +++ b/drivers/media/platform/vsp1/vsp1_regs.h
 @@ -0,0 +1,581 @@
 +/*
 + * vsp1_regs.h  --  R-Car VSP1 Registers Definitions
 + *
 + * Copyright (C) 2013 Renesas Electronics Corporation
 + *
 + * Contact: Laurent Pinchart (laurent.pinch...@ideasonboard.com)
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2
 + * as published by the Free Software Foundation.
 + */
 +
 +#ifndef __VSP1_REGS_H__
 +#define __VSP1_REGS_H__
 +
(snip)
 +/* 
 -
 + * HGO Control Registers
 + */
 +
 +#define VI6_HGO_OFFSET   0x3000
 +#define VI6_HGO_SIZE 0x3004
 +#define VI6_HGO_MODE 0x3008
 +#define VI6_HGO_LB_TH0x300c
 +#define VI6_HGO_LBn_H(0x3010 + (n) * 8)
 +#define VI6_HGO_LBn_V(0x3014 + (n) * 8)

It looks like the 'n' argument is missing for VI6_HGO_LBn_H
and VI6_HGO_LBn_V.

 +#define VI6_HGO_R_HISTO  0x3030
 +#define VI6_HGO_R_MAXMIN 0x3130
 +#define VI6_HGO_R_SUM0x3134
 +#define VI6_HGO_R_LB_DET 0x3138
 +#define VI6_HGO_G_HISTO  0x3140
 +#define VI6_HGO_G_MAXMIN 0x3240
 +#define VI6_HGO_G_SUM0x3244
 +#define VI6_HGO_G_LB_DET 0x3248
 +#define VI6_HGO_B_HISTO  0x3250
 +#define VI6_HGO_B_MAXMIN 0x3350
 +#define VI6_HGO_B_SUM0x3354
 +#define VI6_HGO_B_LB_DET 0x3358
 +#define VI6_HGO_REGRST   0x33fc
 +
 +/* 
 

Re: [PATCH v2 5/5] v4l: Renesas R-Car VSP1 driver

2013-07-24 Thread Laurent Pinchart
Hi Matsubara-san,

On Wednesday 24 July 2013 19:38:39 Katsuya MATSUBARA wrote:
 Hi Laurent,
 
 Thank you for your great work for VSP1.
 Some comments below.

Thank you for the review.

 From: Laurent Pinchart laurent.pinchart+rene...@ideasonboard.com
 Date: Wed, 17 Jul 2013 16:54:42 +0200
 
 (snip)
 
  +++ b/drivers/media/platform/vsp1/vsp1_drv.c
  +/*
  + * vsp1_drv.c  --  R-Car VSP1 Driver
  + *
  + * Copyright (C) 2013 Renesas Corporation
  + *
  + * Contact: Laurent Pinchart (laurent.pinch...@ideasonboard.com)
  + *
  + * This program is free software; you can redistribute it and/or modify
  + * it under the terms of the GNU General Public License as published by
  + * the Free Software Foundation; either version 2 of the License, or
  + * (at your option) any later version.
  + */
  +
  +#include linux/clk.h
  +#include linux/delay.h
  +#include linux/device.h
  +#include linux/interrupt.h
  +#include linux/module.h
  +#include linux/platform_device.h
  +#include linux/videodev2.h
  +
  +#include vsp1.h
  +#include vsp1_lif.h
  +#include vsp1_rwpf.h
  +#include vsp1_uds.h
  +
  +/*
  -
   + * Interrupt Handling
  + */
  +
  +static irqreturn_t vsp1_irq_handler(int irq, void *data)
  +{
  +   struct vsp1_device *vsp1 = data;
  +   irqreturn_t ret = IRQ_NONE;
  +   unsigned int i;
  +
  +   for (i = 0; i  VPS1_MAX_WPF; ++i) {
  +   struct vsp1_rwpf *wpf = vsp1-wpf[i];
  +   struct vsp1_pipeline *pipe;
  +   u32 status;
  +
  +   if (wpf == NULL)
  +   continue;
  +
  +   pipe = to_vsp1_pipeline(wpf-entity.subdev.entity);
  +   status = vsp1_read(vsp1, VI6_WPF_IRQ_STA(i));
  +   vsp1_write(vsp1, VI6_WPF_IRQ_STA(i), ~status);
 
 The value set into the VI6_WPF_IRQ_STA register should be masked with
 VI6_WFP_IRQ_STA_FRE since unused (upper) bits in the register must be
 written with 0.

Good point. I'll fix that.

  +static int vsp1_create_links(struct vsp1_device *vsp1, struct vsp1_entity
  *sink) +{
  +   struct media_entity *entity = sink-subdev.entity;
  +   struct vsp1_entity *source;
  +   unsigned int pad;
  +   int ret;
  +
  +   list_for_each_entry(source, vsp1-entities, list_dev) {
  +   u32 flags;
  +
  +   if (source-type == sink-type)
  +   continue;
  +
  +   if (source-type == VSP1_ENTITY_LIF ||
  +   source-type == VSP1_ENTITY_WPF)
  +   continue;
  +
  +   flags = source-type == VSP1_ENTITY_RPF 
  +   sink-type == VSP1_ENTITY_WPF 
  +   source-index == sink-index
  + ? MEDIA_LNK_FL_ENABLED : 0;
  +
  +   for (pad = 0; pad  entity-num_pads; ++pad) {
  +   if (!(entity-pads[pad].flags  MEDIA_PAD_FL_SINK))
  +   continue;
  +
  +   ret = media_entity_create_link(source-subdev.entity,
  +  source-source_pad,
  +  entity, pad, flags);
  +   if (ret  0)
  +   return ret;
 
 This initialization enables some of links as the initial status.
 I think link_setup() for each linked entity should be invoked here
 to set up the sink value in the vsp_entity structure.
 
 (snip)
 
  +++ b/drivers/media/platform/vsp1/vsp1_regs.h
  @@ -0,0 +1,581 @@
  +/*
  + * vsp1_regs.h  --  R-Car VSP1 Registers Definitions
  + *
  + * Copyright (C) 2013 Renesas Electronics Corporation
  + *
  + * Contact: Laurent Pinchart (laurent.pinch...@ideasonboard.com)
  + *
  + * This program is free software; you can redistribute it and/or modify
  + * it under the terms of the GNU General Public License version 2
  + * as published by the Free Software Foundation.
  + */
  +
  +#ifndef __VSP1_REGS_H__
  +#define __VSP1_REGS_H__
  +
 
 (snip)
 
  +/*
  -
   + * HGO Control Registers
  + */
  +
  +#define VI6_HGO_OFFSET 0x3000
  +#define VI6_HGO_SIZE   0x3004
  +#define VI6_HGO_MODE   0x3008
  +#define VI6_HGO_LB_TH  0x300c
  +#define VI6_HGO_LBn_H  (0x3010 + (n) * 8)
  +#define VI6_HGO_LBn_V  (0x3014 + (n) * 8)
 
 It looks like the 'n' argument is missing for VI6_HGO_LBn_H
 and VI6_HGO_LBn_V.

Will fix as well.

  +#define VI6_HGO_R_HISTO0x3030
  +#define VI6_HGO_R_MAXMIN   0x3130
  +#define VI6_HGO_R_SUM  0x3134
  +#define VI6_HGO_R_LB_DET   0x3138
  +#define VI6_HGO_G_HISTO0x3140
  +#define VI6_HGO_G_MAXMIN   0x3240
  +#define VI6_HGO_G_SUM  0x3244
  +#define VI6_HGO_G_LB_DET   0x3248
  +#define VI6_HGO_B_HISTO0x3250
  +#define VI6_HGO_B_MAXMIN 

Re: [PATCH v2 5/5] v4l: Renesas R-Car VSP1 driver

2013-07-24 Thread Sakari Ailus
Hi Laurent,

What a nice driver! A few minor comments below:

On Wed, Jul 17, 2013 at 04:54:42PM +0200, Laurent Pinchart wrote:
...
 +static void vsp1_device_init(struct vsp1_device *vsp1)
 +{
 + unsigned int i;
 + u32 status;
 +
 + /* Reset any channel that might be running. */
 + status = vsp1_read(vsp1, VI6_STATUS);
 +
 + for (i = 0; i  VPS1_MAX_WPF; ++i) {
 + unsigned int timeout;
 +
 + if (!(status  VI6_STATUS_SYS_ACT(i)))
 + continue;
 +
 + vsp1_write(vsp1, VI6_SRESET, VI6_SRESET_SRTS(i));
 + for (timeout = 10; timeout  0; --timeout) {
 + status = vsp1_read(vsp1, VI6_STATUS);
 + if (!(status  VI6_STATUS_SYS_ACT(i)))
 + break;
 +
 + usleep_range(1000, 2000);
 + }
 +
 + if (timeout)
 + dev_err(vsp1-dev, failed to reset wpf.%u\n, i);

Have you seen this happening in practice? Do you expect the device to
function if resetting it fails?

 + }
 +
 + vsp1_write(vsp1, VI6_CLK_DCSWT, (8  VI6_CLK_DCSWT_CSTPW_SHIFT) |
 +(8  VI6_CLK_DCSWT_CSTRW_SHIFT));
 +
 + for (i = 0; i  VPS1_MAX_RPF; ++i)
 + vsp1_write(vsp1, VI6_DPR_RPF_ROUTE(i), VI6_DPR_NODE_UNUSED);
 +
 + for (i = 0; i  VPS1_MAX_UDS; ++i)
 + vsp1_write(vsp1, VI6_DPR_UDS_ROUTE(i), VI6_DPR_NODE_UNUSED);
 +
 + vsp1_write(vsp1, VI6_DPR_SRU_ROUTE, VI6_DPR_NODE_UNUSED);
 + vsp1_write(vsp1, VI6_DPR_LUT_ROUTE, VI6_DPR_NODE_UNUSED);
 + vsp1_write(vsp1, VI6_DPR_CLU_ROUTE, VI6_DPR_NODE_UNUSED);
 + vsp1_write(vsp1, VI6_DPR_HST_ROUTE, VI6_DPR_NODE_UNUSED);
 + vsp1_write(vsp1, VI6_DPR_HSI_ROUTE, VI6_DPR_NODE_UNUSED);
 + vsp1_write(vsp1, VI6_DPR_BRU_ROUTE, VI6_DPR_NODE_UNUSED);
 +
 + vsp1_write(vsp1, VI6_DPR_HGO_SMPPT, (7  VI6_DPR_SMPPT_TGW_SHIFT) |
 +(VI6_DPR_NODE_UNUSED  VI6_DPR_SMPPT_PT_SHIFT));
 + vsp1_write(vsp1, VI6_DPR_HGT_SMPPT, (7  VI6_DPR_SMPPT_TGW_SHIFT) |
 +(VI6_DPR_NODE_UNUSED  VI6_DPR_SMPPT_PT_SHIFT));
 +}

...

 +int vsp1_entity_init(struct vsp1_device *vsp1, struct vsp1_entity *entity,
 +  unsigned int num_pads)
 +{
 + static const struct {
 + unsigned int id;
 + unsigned int reg;
 + } routes[] = {
 + { VI6_DPR_NODE_LIF, 0 },
 + { VI6_DPR_NODE_RPF(0), VI6_DPR_RPF_ROUTE(0) },
 + { VI6_DPR_NODE_RPF(1), VI6_DPR_RPF_ROUTE(1) },
 + { VI6_DPR_NODE_RPF(2), VI6_DPR_RPF_ROUTE(2) },
 + { VI6_DPR_NODE_RPF(3), VI6_DPR_RPF_ROUTE(3) },
 + { VI6_DPR_NODE_RPF(4), VI6_DPR_RPF_ROUTE(4) },
 + { VI6_DPR_NODE_UDS(0), VI6_DPR_UDS_ROUTE(0) },
 + { VI6_DPR_NODE_UDS(1), VI6_DPR_UDS_ROUTE(1) },
 + { VI6_DPR_NODE_UDS(2), VI6_DPR_UDS_ROUTE(2) },
 + { VI6_DPR_NODE_WPF(0), 0 },
 + { VI6_DPR_NODE_WPF(1), 0 },
 + { VI6_DPR_NODE_WPF(2), 0 },
 + { VI6_DPR_NODE_WPF(3), 0 },
 + };
 +
 + unsigned int i;
 + int ret;
 +
 + for (i = 0; i  ARRAY_SIZE(routes); ++i) {
 + if (routes[i].id == entity-id) {
 + entity-route = routes[i].reg;
 + break;
 + }
 + }
 +
 + if (i == ARRAY_SIZE(routes))
 + return -EINVAL;
 +
 + entity-vsp1 = vsp1;
 + entity-source_pad = num_pads - 1;
 +
 + /* Allocate formats and pads. */
 + entity-formats = devm_kzalloc(vsp1-dev,
 +num_pads * sizeof(*entity-formats),
 +GFP_KERNEL);
 + if (entity-formats == NULL)
 + return -ENOMEM;
 +
 + entity-pads = devm_kzalloc(vsp1-dev, num_pads * sizeof(*entity-pads),
 + GFP_KERNEL);
 + if (entity-pads == NULL)
 + return -ENOMEM;
 +
 + /* Initialize pads. */
 + for (i = 0; i  num_pads - 1; ++i)
 + entity-pads[i].flags = MEDIA_PAD_FL_SINK;
 +
 + entity-pads[num_pads - 1].flags = MEDIA_PAD_FL_SOURCE;
 +
 + /* Initialize the media entity. */
 + ret = media_entity_init(entity-subdev.entity, num_pads,
 + entity-pads, 0);

How about return media_entity_init(...) instead?

 + if (ret  0)
 + return ret;
 +
 + return 0;
 +}

...

 +static int lif_enum_mbus_code(struct v4l2_subdev *subdev,
 +   struct v4l2_subdev_fh *fh,
 +   struct v4l2_subdev_mbus_code_enum *code)
 +{
 + static const unsigned int codes[] = {
 + V4L2_MBUS_FMT_ARGB_1X32,
 + V4L2_MBUS_FMT_AYUV8_1X32,
 + };
 + struct v4l2_mbus_framefmt *format;
 +
 + if (code-pad == LIF_PAD_SINK) {
 + if (code-index = ARRAY_SIZE(codes))
 + return -EINVAL;
 +
 + code-code = codes[code-index];
 + }