Hi Sakari,

-----Original Message-----
From: Sakari Ailus [mailto:sakari.ailus@iki.fi]
Sent: Tuesday, June 18, 2019 8:29 PM
To: Vishal Sagar <redacted>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>; Vishal Sagar
[off-list ref]; Hyun Kwon [off-list ref];
laurent.pinchart@ideasonboard.com; mchehab@kernel.org;
robh+dt@kernel.org; mark.rutland@arm.com; Michal Simek
[off-list ref]; linux-media@vger.kernel.org;
devicetree@vger.kernel.org; hans.verkuil@cisco.com; linux-arm-
kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Dinesh Kumar
[off-list ref]; Sandip Kothari [off-list ref]; Luca Ceresoli
[off-list ref]; Jacopo Mondi [off-list ref]
Subject: Re: [PATCH v8 2/2] media: v4l: xilinx: Add Xilinx MIPI CSI-2 Rx
Subsystem driver

Hi Vishal,

On Fri, Jun 07, 2019 at 07:11:47AM +0000, Vishal Sagar wrote:
...

quoted

quoted

quoted

+/**
+ * xcsi2rxss_s_ctrl - This is used to set the Xilinx MIPI CSI-2 V4L2 controls
+ * @ctrl: V4L2 control to be set
+ *
+ * This function is used to set the V4L2 controls for the Xilinx MIPI
+ * CSI-2 Rx Subsystem. It is used to set the active lanes in the system.
+ * The event counters can be reset.
+ *
+ * Return: 0 on success, errors otherwise
+ */
+static int xcsi2rxss_s_ctrl(struct v4l2_ctrl *ctrl)
+{
+     struct xcsi2rxss_state *xcsi2rxss =
+             container_of(ctrl->handler, struct xcsi2rxss_state,
+                          ctrl_handler);
+     struct xcsi2rxss_core *core = &xcsi2rxss->core;
+     int ret = 0;
+
+     mutex_lock(&xcsi2rxss->lock);
+
+     switch (ctrl->id) {
+     case V4L2_CID_XILINX_MIPICSISS_ACT_LANES:
+             /*
+              * This will be called only when "Enable Active Lanes" parameter
+              * is set in design
+              */

You generally get the number of lanes from firmware. There's no need to

add

quoted

quoted

a control for it.

I don't understand what firmware means here. There is no other code

running.

quoted

I don't see how to modify the number of lanes apart from using v4l control.

It's not the user that provides this information. Again, if you want this
feature right from the time the driver is merged to mainline, then rebase
the set on top of Jacopo's frame descriptor set. But it may take a while.

Thanks for reviewing again and sharing this. 
Since Jacopo's frame descriptor set will take a while, I will remove this control for now from the driver so that the driver can get into upstream.

Regards
Vishal Sagar

quoted

quoted

quoted

+             if (core->enable_active_lanes) {
+                     u32 active_lanes;
+
+                     xcsi2rxss_clr_and_set(core, XCSI_PCR_OFFSET,
+                                           XCSI_PCR_ACTLANES_MASK,
+                                           ctrl->val - 1);
+                     /*
+                      * This delay is to allow the value to reflect as write
+                      * and read paths are different.
+                      */
+                     udelay(1);
+                     active_lanes = xcsi2rxss_read(core, XCSI_PCR_OFFSET);
+                     active_lanes &= XCSI_PCR_ACTLANES_MASK;
+                     active_lanes++;
+                     if (active_lanes != ctrl->val)
+                             dev_info(core->dev, "RxByteClkHS absent\n");
+                     dev_dbg(core->dev, "active lanes = %d\n", ctrl->val);
+             } else {
+                     ret = -EINVAL;
+             }
+             break;
+     case V4L2_CID_XILINX_MIPICSISS_RESET_COUNTERS:
+             xcsi2rxss_reset_event_counters(xcsi2rxss);
+             break;
+     default:
+             ret = -EINVAL;
+             break;
+     }
+
+     mutex_unlock(&xcsi2rxss->lock);
+
+     return ret;
+}

--
Regards,

Sakari Ailus

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel