[PATCH 00/15] omap3isp driver DT support

[PATCH 00/15] omap3isp driver DT support

[Sakari Ailus]

Hi folks,

Here's the first non-RFC version of the omap3isp driver DT support patchset.
It's a sub-set of patches in this RFC set:

<URL:http://www.spinics.net/lists/linux-media/msg87263.html>

What has been changed since the RFC set in these patches is roughly:

- The patches have been rebased on Laurent's hist DMA patch:

  <URL:http://www.spinics.net/lists/linux-media/msg87349.html>

- the dts changes have been split off as a separate set "[PATCH 0/4] OMAP 3
  ISP (and N9/N950 primary camera support) dts changes". This includes the
  OMAP 3 ISP DT binding patch. There are no direct dependencies to this set.

- Use unsigned int rather than int in loop over an array in v4l2-of.c.

- Check that there at least as as many lane polarities as there are lanes.

- Fixed syscon handling for non-DT case.

-- 
Kind regards,
Sakari

[PATCH 01/15] omap3isp: Fix error handling in probe

[Sakari Ailus]

The mutex was not destroyed correctly if dma_coerce_mask_and_coherent()
failed for some reason.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/omap3isp/isp.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index deca809..fb193b6 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2252,7 +2252,7 @@ static int isp_probe(struct platform_device *pdev)
 
 	ret = dma_coerce_mask_and_coherent(isp->dev, DMA_BIT_MASK(32));
 	if (ret)
-		return ret;
+		goto error;
 
 	platform_set_drvdata(pdev, isp);
 
-- 
1.7.10.4

[PATCH 02/15] omap3isp: Avoid a BUG_ON() in media_entity_create_link()

[Sakari Ailus]

If an uninitialised v4l2_subdev struct was passed to
media_entity_create_link(), one of the BUG_ON()'s in the function will be
hit since media_entity.num_pads will be zero. Avoid this by checking whether
the num_pads field is non-zero for the interface.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/omap3isp/isp.c |   13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index fb193b6..4ab674d 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -1946,6 +1946,19 @@ static int isp_register_entities(struct isp_device *isp)
 			goto done;
 		}
 
+		/*
+		 * Not all interfaces are available on all revisions
+		 * of the ISP. The sub-devices of those interfaces
+		 * aren't initialised in such a case. Check this by
+		 * ensuring the num_pads is non-zero.
+		 */
+		if (!input->num_pads) {
+			dev_err(isp->dev, "%s: invalid input %u\n",
+				entity->name, subdevs->interface);
+			ret = -EINVAL;
+			goto done;
+		}
+
 		for (i = 0; i < sensor->entity.num_pads; i++) {
 			if (sensor->entity.pads[i].flags & MEDIA_PAD_FL_SOURCE)
 				break;
-- 
1.7.10.4

[PATCH 03/15] omap3isp: Separate external link creation from platform data parsing

[Sakari Ailus]

Move the code which connects the external entity to an ISP entity into a
separate function. This disconnects it from parsing the platform data.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/omap3isp/isp.c |  143 +++++++++++++++++----------------
 1 file changed, 72 insertions(+), 71 deletions(-)

diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 4ab674d..f694615 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -1832,6 +1832,77 @@ isp_register_subdev_group(struct isp_device *isp,
 	return sensor;
 }
 
+static int isp_link_entity(
+	struct isp_device *isp, struct media_entity *entity,
+	enum isp_interface_type interface)
+{
+	struct media_entity *input;
+	unsigned int flags;
+	unsigned int pad;
+	unsigned int i;
+
+	/* Connect the sensor to the correct interface module.
+	 * Parallel sensors are connected directly to the CCDC, while
+	 * serial sensors are connected to the CSI2a, CCP2b or CSI2c
+	 * receiver through CSIPHY1 or CSIPHY2.
+	 */
+	switch (interface) {
+	case ISP_INTERFACE_PARALLEL:
+		input = &isp->isp_ccdc.subdev.entity;
+		pad = CCDC_PAD_SINK;
+		flags = 0;
+		break;
+
+	case ISP_INTERFACE_CSI2A_PHY2:
+		input = &isp->isp_csi2a.subdev.entity;
+		pad = CSI2_PAD_SINK;
+		flags = MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED;
+		break;
+
+	case ISP_INTERFACE_CCP2B_PHY1:
+	case ISP_INTERFACE_CCP2B_PHY2:
+		input = &isp->isp_ccp2.subdev.entity;
+		pad = CCP2_PAD_SINK;
+		flags = 0;
+		break;
+
+	case ISP_INTERFACE_CSI2C_PHY1:
+		input = &isp->isp_csi2c.subdev.entity;
+		pad = CSI2_PAD_SINK;
+		flags = MEDIA_LNK_FL_IMMUTABLE | MEDIA_LNK_FL_ENABLED;
+		break;
+
+	default:
+		dev_err(isp->dev, "%s: invalid interface type %u\n", __func__,
+			interface);
+		return -EINVAL;
+	}
+
+	/*
+	 * Not all interfaces are available on all revisions of the
+	 * ISP. The sub-devices of those interfaces aren't initialised
+	 * in such a case. Check this by ensuring the num_pads is
+	 * non-zero.
+	 */
+	if (!input->num_pads) {
+		dev_err(isp->dev, "%s: invalid input %u\n", entity->name,
+			interface);
+		return -EINVAL;
+	}
+
+	for (i = 0; i < entity->num_pads; i++) {
+		if (entity->pads[i].flags & MEDIA_PAD_FL_SOURCE)
+			break;
+	}
+	if (i == entity->num_pads) {
+		dev_err(isp->dev, "%s: no source pad in external entity\n",
+			__func__);
+		return -EINVAL;
+	}
+
+	return media_entity_create_link(entity, i, input, pad, flags);
+}
+
 static int isp_register_entities(struct isp_device *isp)
 {
 	struct isp_platform_data *pdata = isp->pdata;
@@ -1895,10 +1966,6 @@ static int isp_register_entities(struct isp_device *isp)
 	/* Register external entities */
 	for (subdevs = pdata->subdevs; subdevs && subdevs->subdevs; ++subdevs) {
 		struct v4l2_subdev *sensor;
-		struct media_entity *input;
-		unsigned int flags;
-		unsigned int pad;
-		unsigned int i;
 
 		sensor = isp_register_subdev_group(isp, subdevs->subdevs);
 		if (sensor == NULL)
@@ -1906,73 +1973,7 @@ static int isp_register_entities(struct isp_device *isp)
 
 		sensor->host_priv = subdevs;
 
-		/* Connect the sensor to the correct interface module. Parallel
-		 * sensors are connected directly to the CCDC, while serial
-		 * sensors are connected to the CSI2a, CCP2b or CSI2c receiver
-		 * through CSIPHY1 or CSIPHY2.
-		 */
-		switch (subdevs->interface) {
-		case ISP_INTERFACE_PARALLEL:
-			input = &isp->isp_ccdc.subdev.entity;
-			pad = CCDC_PAD_SINK;
-			flags = 0;
-			break;
-
-		case ISP_INTERFACE_CSI2A_PHY2:
-			input = &isp->isp_csi2a.subdev.entity;
-			pad = CSI2_PAD_SINK;
-			flags = MEDIA_LNK_FL_IMMUTABLE
-			      | MEDIA_LNK_FL_ENABLED;
-			break;
-
-		case ISP_INTERFACE_CCP2B_PHY1:
-		case ISP_INTERFACE_CCP2B_PHY2:
-			input = &isp->isp_ccp2.subdev.entity;
-			pad = CCP2_PAD_SINK;
-			flags = 0;
-			break;
-
-		case ISP_INTERFACE_CSI2C_PHY1:
-			input = &isp->isp_csi2c.subdev.entity;
-			pad = CSI2_PAD_SINK;
-			flags = MEDIA_LNK_FL_IMMUTABLE
-			      | MEDIA_LNK_FL_ENABLED;
-			break;
-
-		default:
-			dev_err(isp->dev, "%s: invalid interface type %u\n",
-				__func__, subdevs->interface);
-			ret = -EINVAL;
-			goto done;
-		}
-
-		/*
-		 * Not all interfaces are available on all revisions
-		 * of the ISP. The sub-devices of those interfaces
-		 * aren't initialised in such a case. Check this by
-		 * ensuring the num_pads is non-zero.
-		 */
-		if (!input->num_pads) {
-			dev_err(isp->dev, "%s: invalid input %u\n",
-				entity->name, subdevs->interface);
-			ret = -EINVAL;
-			goto done;
-		}
-
-		for (i = 0; i < sensor->entity.num_pads; i++) {
-			if (sensor->entity.pads[i].flags & MEDIA_PAD_FL_SOURCE)
-				break;
-		}
-		if (i == sensor->entity.num_pads) {
-			dev_err(isp->dev,
-				"%s: no source pad in external entity\n",
-				__func__);
-			ret = -EINVAL;
-			goto done;
-		}
-
-		ret = media_entity_create_link(&sensor->entity, i, input, pad,
-					       flags);
+		ret = isp_link_entity(isp, &sensor->entity, subdevs->interface);
 		if (ret < 0)
 			goto done;
 	}
-- 
1.7.10.4

[PATCH 04/15] omap3isp: DT support for clocks

[Sakari Ailus]

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/omap3isp/isp.c |   25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index f694615..82499cd 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -279,9 +279,21 @@ static const struct clk_init_data isp_xclk_init_data = {
 	.num_parents = 1,
 };
 
+static struct clk *isp_xclk_src_get(struct of_phandle_args *clkspec, void *data)
+{
+	unsigned int idx = clkspec->args[0];
+	struct isp_device *isp = data;
+
+	if (idx >= ARRAY_SIZE(isp->xclks))
+		return ERR_PTR(-ENOENT);
+
+	return isp->xclks[idx].clk;
+}
+
 static int isp_xclk_init(struct isp_device *isp)
 {
 	struct isp_platform_data *pdata = isp->pdata;
+	struct device_node *np = isp->dev->of_node;
 	struct clk_init_data init;
 	unsigned int i;
 
@@ -312,6 +324,12 @@ static int isp_xclk_init(struct isp_device *isp)
 		if (IS_ERR(xclk->clk))
 			return PTR_ERR(xclk->clk);
 
+		/* When instantiated from DT we don't need to register clock
+		 * aliases.
+		 */
+		if (np)
+			continue;
+
 		if (pdata->xclks[i].con_id == NULL &&
 		    pdata->xclks[i].dev_id == NULL)
 			continue;
@@ -327,13 +345,20 @@ static int isp_xclk_init(struct isp_device *isp)
 		clkdev_add(xclk->lookup);
 	}
 
+	if (np)
+		of_clk_add_provider(np, isp_xclk_src_get, isp);
+
 	return 0;
 }
 
 static void isp_xclk_cleanup(struct isp_device *isp)
 {
+	struct device_node *np = isp->dev->of_node;
 	unsigned int i;
 
+	if (np)
+		of_clk_del_provider(np);
+
 	for (i = 0; i < ARRAY_SIZE(isp->xclks); ++i) {
 		struct isp_xclk *xclk = &isp->xclks[i];
 
-- 
1.7.10.4

[PATCH 05/15] omap3isp: Platform data could be NULL

[Sakari Ailus]

Only check for call platform data callback functions if there's platform
data. Also take care of a few other cases where the NULL pdata pointer could
have been accessed, and remove the check for NULL dev->platform_data
pointer.

Removing the check for NULL dev->platform_data isn't strictly needed by the
DT support but there's no harm from that either: the device now can be used
without sensors, for instance.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/omap3isp/isp.c      |   10 ++++------
 drivers/media/platform/omap3isp/ispvideo.c |    6 +++---
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 82499cd..537377b 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -330,8 +330,8 @@ static int isp_xclk_init(struct isp_device *isp)
 		if (np)
 			continue;
 
-		if (pdata->xclks[i].con_id == NULL &&
-		    pdata->xclks[i].dev_id == NULL)
+		if (!pdata || (pdata->xclks[i].con_id == NULL &&
+			       pdata->xclks[i].dev_id == NULL))
 			continue;
 
 		xclk->lookup = kzalloc(sizeof(*xclk->lookup), GFP_KERNEL);
@@ -1989,7 +1989,8 @@ static int isp_register_entities(struct isp_device *isp)
 		goto done;
 
 	/* Register external entities */
-	for (subdevs = pdata->subdevs; subdevs && subdevs->subdevs; ++subdevs) {
+	for (subdevs = pdata ? pdata->subdevs : NULL;
+	     subdevs && subdevs->subdevs; ++subdevs) {
 		struct v4l2_subdev *sensor;
 
 		sensor = isp_register_subdev_group(isp, subdevs->subdevs);
@@ -2271,9 +2272,6 @@ static int isp_probe(struct platform_device *pdev)
 	int ret;
 	int i, m;
 
-	if (pdata == NULL)
-		return -EINVAL;
-
 	isp = devm_kzalloc(&pdev->dev, sizeof(*isp), GFP_KERNEL);
 	if (!isp) {
 		dev_err(&pdev->dev, "could not allocate memory\n");
diff --git a/drivers/media/platform/omap3isp/ispvideo.c b/drivers/media/platform/omap3isp/ispvideo.c
index 0b5967e..d285af1 100644
--- a/drivers/media/platform/omap3isp/ispvideo.c
+++ b/drivers/media/platform/omap3isp/ispvideo.c
@@ -1018,7 +1018,7 @@ isp_video_streamon(struct file *file, void *fh, enum v4l2_buf_type type)
 
 	pipe->entities = 0;
 
-	if (video->isp->pdata->set_constraints)
+	if (video->isp->pdata && video->isp->pdata->set_constraints)
 		video->isp->pdata->set_constraints(video->isp, true);
 	pipe->l3_ick = clk_get_rate(video->isp->clock[ISP_CLK_L3_ICK]);
 	pipe->max_rate = pipe->l3_ick;
@@ -1100,7 +1100,7 @@ err_set_stream:
 err_check_format:
 	media_entity_pipeline_stop(&video->video.entity);
 err_pipeline_start:
-	if (video->isp->pdata->set_constraints)
+	if (video->isp->pdata && video->isp->pdata->set_constraints)
 		video->isp->pdata->set_constraints(video->isp, false);
 	/* The DMA queue must be emptied here, otherwise CCDC interrupts that
 	 * will get triggered the next time the CCDC is powered up will try to
@@ -1161,7 +1161,7 @@ isp_video_streamoff(struct file *file, void *fh, enum v4l2_buf_type type)
 	video->queue = NULL;
 	video->error = false;
 
-	if (video->isp->pdata->set_constraints)
+	if (video->isp->pdata && video->isp->pdata->set_constraints)
 		video->isp->pdata->set_constraints(video->isp, false);
 	media_entity_pipeline_stop(&video->video.entity);
 
-- 
1.7.10.4

[PATCH 06/15] omap3isp: Refactor device configuration structs for Device Tree

[Sakari Ailus]

Make omap3isp configuration data structures more suitable for consumption by
the DT by separating the I2C bus information of all the sub-devices in a
group and the ISP bus information from each other. The ISP bus information
is made a pointer instead of being directly embedded in the struct.

In the case of the DT only the sensor specific information on the ISP bus
configuration is retained. The structs are renamed to reflect that.

After this change the structs needed to describe device configuration can be
allocated and accessed separately without those needed only in the case of
platform data. The platform data related structs can be later removed once
the support for platform data can be removed.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Igor Grinberg <grinberg@compulab.co.il>
---
 arch/arm/mach-omap2/board-cm-t35.c          |   57 +++++++-----------
 drivers/media/platform/omap3isp/isp.c       |   86 +++++++++++++--------------
 drivers/media/platform/omap3isp/isp.h       |    2 +-
 drivers/media/platform/omap3isp/ispccdc.c   |   26 ++++----
 drivers/media/platform/omap3isp/ispccp2.c   |   22 +++----
 drivers/media/platform/omap3isp/ispcsi2.c   |    8 +--
 drivers/media/platform/omap3isp/ispcsiphy.c |   21 ++++---
 include/media/omap3isp.h                    |   34 +++++------
 8 files changed, 119 insertions(+), 137 deletions(-)

diff --git a/arch/arm/mach-omap2/board-cm-t35.c b/arch/arm/mach-omap2/board-cm-t35.c
index 91738a1..b5dfbc1 100644
--- a/arch/arm/mach-omap2/board-cm-t35.c
+++ b/arch/arm/mach-omap2/board-cm-t35.c
@@ -492,51 +492,36 @@ static struct twl4030_platform_data cm_t35_twldata = {
 #include <media/omap3isp.h>
 #include "devices.h"
 
-static struct i2c_board_info cm_t35_isp_i2c_boardinfo[] = {
+static struct isp_platform_subdev cm_t35_isp_subdevs[] = {
 	{
-		I2C_BOARD_INFO("mt9t001", 0x5d),
-	},
-	{
-		I2C_BOARD_INFO("tvp5150", 0x5c),
-	},
-};
-
-static struct isp_subdev_i2c_board_info cm_t35_isp_primary_subdevs[] = {
-	{
-		.board_info = &cm_t35_isp_i2c_boardinfo[0],
-		.i2c_adapter_id = 3,
-	},
-	{ NULL, 0, },
-};
-
-static struct isp_subdev_i2c_board_info cm_t35_isp_secondary_subdevs[] = {
-	{
-		.board_info = &cm_t35_isp_i2c_boardinfo[1],
+		.board_info = &(struct i2c_board_info){
+			I2C_BOARD_INFO("mt9t001", 0x5d)
+		},
 		.i2c_adapter_id = 3,
-	},
-	{ NULL, 0, },
-};
-
-static struct isp_v4l2_subdevs_group cm_t35_isp_subdevs[] = {
-	{
-		.subdevs = cm_t35_isp_primary_subdevs,
-		.interface = ISP_INTERFACE_PARALLEL,
-		.bus = {
-			.parallel = {
-				.clk_pol = 1,
+		.bus = &(struct isp_bus_cfg){
+			.interface = ISP_INTERFACE_PARALLEL,
+			.bus = {
+				.parallel = {
+					.clk_pol = 1,
+				},
 			},
 		},
 	},
 	{
-		.subdevs = cm_t35_isp_secondary_subdevs,
-		.interface = ISP_INTERFACE_PARALLEL,
-		.bus = {
-			.parallel = {
-				.clk_pol = 0,
+		.board_info = &(struct i2c_board_info){
+			I2C_BOARD_INFO("tvp5150", 0x5c),
+		},
+		.i2c_adapter_id = 3,
+		.bus = &(struct isp_bus_cfg){
+			.interface = ISP_INTERFACE_PARALLEL,
+			.bus = {
+				.parallel = {
+					.clk_pol = 0,
+				},
 			},
 		},
 	},
-	{ NULL, 0, },
+	{ 0 },
 };
 
 static struct isp_platform_data cm_t35_isp_pdata = {
diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 537377b..1b5c6df 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -447,7 +447,7 @@ static void isp_core_init(struct isp_device *isp, int idle)
  */
 void omap3isp_configure_bridge(struct isp_device *isp,
 			       enum ccdc_input_entity input,
-			       const struct isp_parallel_platform_data *pdata,
+			       const struct isp_parallel_cfg *parcfg,
 			       unsigned int shift, unsigned int bridge)
 {
 	u32 ispctrl_val;
@@ -462,8 +462,8 @@ void omap3isp_configure_bridge(struct isp_device *isp,
 	switch (input) {
 	case CCDC_INPUT_PARALLEL:
 		ispctrl_val |= ISPCTRL_PAR_SER_CLK_SEL_PARALLEL;
-		ispctrl_val |= pdata->clk_pol << ISPCTRL_PAR_CLK_POL_SHIFT;
-		shift += pdata->data_lane_shift * 2;
+		ispctrl_val |= parcfg->clk_pol << ISPCTRL_PAR_CLK_POL_SHIFT;
+		shift += parcfg->data_lane_shift * 2;
 		break;
 
 	case CCDC_INPUT_CSI2A:
@@ -1809,52 +1809,44 @@ static void isp_unregister_entities(struct isp_device *isp)
 }
 
 /*
- * isp_register_subdev_group - Register a group of subdevices
+ * isp_register_subdev - Register a sub-device
  * @isp: OMAP3 ISP device
- * @board_info: I2C subdevs board information array
+ * @isp_subdev: platform data related to a sub-device
  *
- * Register all I2C subdevices in the board_info array. The array must be
- * terminated by a NULL entry, and the first entry must be the sensor.
+ * Register an I2C sub-device which has not been registered by other
+ * means (such as the Device Tree).
  *
- * Return a pointer to the sensor media entity if it has been successfully
+ * Return a pointer to the sub-device if it has been successfully
  * registered, or NULL otherwise.
  */
 static struct v4l2_subdev *
-isp_register_subdev_group(struct isp_device *isp,
-		     struct isp_subdev_i2c_board_info *board_info)
+isp_register_subdev(struct isp_device *isp,
+		    struct isp_platform_subdev *isp_subdev)
 {
-	struct v4l2_subdev *sensor = NULL;
-	unsigned int first;
+	struct i2c_adapter *adapter;
+	struct v4l2_subdev *sd;
 
-	if (board_info->board_info == NULL)
+	if (isp_subdev->board_info == NULL)
 		return NULL;
 
-	for (first = 1; board_info->board_info; ++board_info, first = 0) {
-		struct v4l2_subdev *subdev;
-		struct i2c_adapter *adapter;
-
-		adapter = i2c_get_adapter(board_info->i2c_adapter_id);
-		if (adapter == NULL) {
-			dev_err(isp->dev, "%s: Unable to get I2C adapter %d for "
-				"device %s\n", __func__,
-				board_info->i2c_adapter_id,
-				board_info->board_info->type);
-			continue;
-		}
-
-		subdev = v4l2_i2c_new_subdev_board(&isp->v4l2_dev, adapter,
-				board_info->board_info, NULL);
-		if (subdev == NULL) {
-			dev_err(isp->dev, "%s: Unable to register subdev %s\n",
-				__func__, board_info->board_info->type);
-			continue;
-		}
+	adapter = i2c_get_adapter(isp_subdev->i2c_adapter_id);
+	if (adapter == NULL) {
+		dev_err(isp->dev,
+			"%s: Unable to get I2C adapter %d for device %s\n",
+			__func__, isp_subdev->i2c_adapter_id,
+			isp_subdev->board_info->type);
+		return NULL;
+	}
 
-		if (first)
-			sensor = subdev;
+	sd = v4l2_i2c_new_subdev_board(&isp->v4l2_dev, adapter,
+				       isp_subdev->board_info, NULL);
+	if (sd == NULL) {
+		dev_err(isp->dev, "%s: Unable to register subdev %s\n",
+			__func__, isp_subdev->board_info->type);
+		return NULL;
 	}
 
-	return sensor;
+	return sd;
 }
 
 static int isp_link_entity(
@@ -1931,7 +1923,7 @@ static int isp_link_entity(
 static int isp_register_entities(struct isp_device *isp)
 {
 	struct isp_platform_data *pdata = isp->pdata;
-	struct isp_v4l2_subdevs_group *subdevs;
+	struct isp_platform_subdev *isp_subdev;
 	int ret;
 
 	isp->media_dev.dev = isp->dev;
@@ -1989,17 +1981,23 @@ static int isp_register_entities(struct isp_device *isp)
 		goto done;
 
 	/* Register external entities */
-	for (subdevs = pdata ? pdata->subdevs : NULL;
-	     subdevs && subdevs->subdevs; ++subdevs) {
-		struct v4l2_subdev *sensor;
+	for (isp_subdev = pdata ? pdata->subdevs : NULL;
+	     isp_subdev && isp_subdev->board_info; isp_subdev++) {
+		struct v4l2_subdev *sd;
 
-		sensor = isp_register_subdev_group(isp, subdevs->subdevs);
-		if (sensor == NULL)
+		sd = isp_register_subdev(isp, isp_subdev);
+
+		/*
+		 * No bus information --- this is either a flash or a
+		 * lens subdev.
+		 */
+		if (!sd || !isp_subdev->bus)
 			continue;
 
-		sensor->host_priv = subdevs;
+		sd->host_priv = isp_subdev->bus;
 
-		ret = isp_link_entity(isp, &sensor->entity, subdevs->interface);
+		ret = isp_link_entity(isp, &sd->entity,
+				      isp_subdev->bus->interface);
 		if (ret < 0)
 			goto done;
 	}
diff --git a/drivers/media/platform/omap3isp/isp.h b/drivers/media/platform/omap3isp/isp.h
index cfdfc87..b932a6f 100644
--- a/drivers/media/platform/omap3isp/isp.h
+++ b/drivers/media/platform/omap3isp/isp.h
@@ -229,7 +229,7 @@ int omap3isp_pipeline_set_stream(struct isp_pipeline *pipe,
 void omap3isp_pipeline_cancel_stream(struct isp_pipeline *pipe);
 void omap3isp_configure_bridge(struct isp_device *isp,
 			       enum ccdc_input_entity input,
-			       const struct isp_parallel_platform_data *pdata,
+			       const struct isp_parallel_cfg *buscfg,
 			       unsigned int shift, unsigned int bridge);
 
 struct isp_device *omap3isp_get(struct isp_device *isp);
diff --git a/drivers/media/platform/omap3isp/ispccdc.c b/drivers/media/platform/omap3isp/ispccdc.c
index 587489a..388f971 100644
--- a/drivers/media/platform/omap3isp/ispccdc.c
+++ b/drivers/media/platform/omap3isp/ispccdc.c
@@ -958,11 +958,11 @@ void omap3isp_ccdc_max_rate(struct isp_ccdc_device *ccdc,
 /*
  * ccdc_config_sync_if - Set CCDC sync interface configuration
  * @ccdc: Pointer to ISP CCDC device.
- * @pdata: Parallel interface platform data (may be NULL)
+ * @parcfg: Parallel interface platform data (may be NULL)
  * @data_size: Data size
  */
 static void ccdc_config_sync_if(struct isp_ccdc_device *ccdc,
-				struct isp_parallel_platform_data *pdata,
+				struct isp_parallel_cfg *parcfg,
 				unsigned int data_size)
 {
 	struct isp_device *isp = to_isp_device(ccdc);
@@ -1000,19 +1000,19 @@ static void ccdc_config_sync_if(struct isp_ccdc_device *ccdc,
 		break;
 	}
 
-	if (pdata && pdata->data_pol)
+	if (parcfg && parcfg->data_pol)
 		syn_mode |= ISPCCDC_SYN_MODE_DATAPOL;
 
-	if (pdata && pdata->hs_pol)
+	if (parcfg && parcfg->hs_pol)
 		syn_mode |= ISPCCDC_SYN_MODE_HDPOL;
 
 	/* The polarity of the vertical sync signal output by the BT.656
 	 * decoder is not documented and seems to be active low.
 	 */
-	if ((pdata && pdata->vs_pol) || ccdc->bt656)
+	if ((parcfg && parcfg->vs_pol) || ccdc->bt656)
 		syn_mode |= ISPCCDC_SYN_MODE_VDPOL;
 
-	if (pdata && pdata->fld_pol)
+	if (parcfg && parcfg->fld_pol)
 		syn_mode |= ISPCCDC_SYN_MODE_FLDPOL;
 
 	isp_reg_writel(isp, syn_mode, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SYN_MODE);
@@ -1115,7 +1115,7 @@ static const u32 ccdc_sgbrg_pattern =
 static void ccdc_configure(struct isp_ccdc_device *ccdc)
 {
 	struct isp_device *isp = to_isp_device(ccdc);
-	struct isp_parallel_platform_data *pdata = NULL;
+	struct isp_parallel_cfg *parcfg = NULL;
 	struct v4l2_subdev *sensor;
 	struct v4l2_mbus_framefmt *format;
 	const struct v4l2_rect *crop;
@@ -1145,7 +1145,7 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
 		if (!ret)
 			ccdc->bt656 = cfg.type == V4L2_MBUS_BT656;
 
-		pdata = &((struct isp_v4l2_subdevs_group *)sensor->host_priv)
+		parcfg = &((struct isp_bus_cfg *)sensor->host_priv)
 			->bus.parallel;
 	}
 
@@ -1175,10 +1175,10 @@ static void ccdc_configure(struct isp_ccdc_device *ccdc)
 	else
 		bridge = ISPCTRL_PAR_BRIDGE_DISABLE;
 
-	omap3isp_configure_bridge(isp, ccdc->input, pdata, shift, bridge);
+	omap3isp_configure_bridge(isp, ccdc->input, parcfg, shift, bridge);
 
 	/* Configure the sync interface. */
-	ccdc_config_sync_if(ccdc, pdata, depth_out);
+	ccdc_config_sync_if(ccdc, parcfg, depth_out);
 
 	syn_mode = isp_reg_readl(isp, OMAP3_ISP_IOMEM_CCDC, ISPCCDC_SYN_MODE);
 
@@ -2417,11 +2417,11 @@ static int ccdc_link_validate(struct v4l2_subdev *sd,
 
 	/* We've got a parallel sensor here. */
 	if (ccdc->input == CCDC_INPUT_PARALLEL) {
-		struct isp_parallel_platform_data *pdata =
-			&((struct isp_v4l2_subdevs_group *)
+		struct isp_parallel_cfg *parcfg =
+			&((struct isp_bus_cfg *)
 			  media_entity_to_v4l2_subdev(link->source->entity)
 			  ->host_priv)->bus.parallel;
-		parallel_shift = pdata->data_lane_shift * 2;
+		parallel_shift = parcfg->data_lane_shift * 2;
 	} else {
 		parallel_shift = 0;
 	}
diff --git a/drivers/media/platform/omap3isp/ispccp2.c b/drivers/media/platform/omap3isp/ispccp2.c
index f4aedb3..374a1f4 100644
--- a/drivers/media/platform/omap3isp/ispccp2.c
+++ b/drivers/media/platform/omap3isp/ispccp2.c
@@ -201,14 +201,14 @@ static void ccp2_mem_enable(struct isp_ccp2_device *ccp2, u8 enable)
 /*
  * ccp2_phyif_config - Initialize CCP2 phy interface config
  * @ccp2: Pointer to ISP CCP2 device
- * @pdata: CCP2 platform data
+ * @buscfg: CCP2 platform data
  *
  * Configure the CCP2 physical interface module from platform data.
  *
  * Returns -EIO if strobe is chosen in CSI1 mode, or 0 on success.
  */
 static int ccp2_phyif_config(struct isp_ccp2_device *ccp2,
-			     const struct isp_ccp2_platform_data *pdata)
+			     const struct isp_ccp2_cfg *buscfg)
 {
 	struct isp_device *isp = to_isp_device(ccp2);
 	u32 val;
@@ -218,16 +218,16 @@ static int ccp2_phyif_config(struct isp_ccp2_device *ccp2,
 			    ISPCCP2_CTRL_IO_OUT_SEL | ISPCCP2_CTRL_MODE;
 	/* Data/strobe physical layer */
 	BIT_SET(val, ISPCCP2_CTRL_PHY_SEL_SHIFT, ISPCCP2_CTRL_PHY_SEL_MASK,
-		pdata->phy_layer);
+		buscfg->phy_layer);
 	BIT_SET(val, ISPCCP2_CTRL_INV_SHIFT, ISPCCP2_CTRL_INV_MASK,
-		pdata->strobe_clk_pol);
+		buscfg->strobe_clk_pol);
 	isp_reg_writel(isp, val, OMAP3_ISP_IOMEM_CCP2, ISPCCP2_CTRL);
 
 	val = isp_reg_readl(isp, OMAP3_ISP_IOMEM_CCP2, ISPCCP2_CTRL);
 	if (!(val & ISPCCP2_CTRL_MODE)) {
-		if (pdata->ccp2_mode == ISP_CCP2_MODE_CCP2)
+		if (buscfg->ccp2_mode == ISP_CCP2_MODE_CCP2)
 			dev_warn(isp->dev, "OMAP3 CCP2 bus not available\n");
-		if (pdata->phy_layer == ISP_CCP2_PHY_DATA_STROBE)
+		if (buscfg->phy_layer == ISP_CCP2_PHY_DATA_STROBE)
 			/* Strobe mode requires CCP2 */
 			return -EIO;
 	}
@@ -347,7 +347,7 @@ static void ccp2_lcx_config(struct isp_ccp2_device *ccp2,
  */
 static int ccp2_if_configure(struct isp_ccp2_device *ccp2)
 {
-	const struct isp_v4l2_subdevs_group *pdata;
+	const struct isp_bus_cfg *buscfg;
 	struct v4l2_mbus_framefmt *format;
 	struct media_pad *pad;
 	struct v4l2_subdev *sensor;
@@ -358,20 +358,20 @@ static int ccp2_if_configure(struct isp_ccp2_device *ccp2)
 
 	pad = media_entity_remote_pad(&ccp2->pads[CCP2_PAD_SINK]);
 	sensor = media_entity_to_v4l2_subdev(pad->entity);
-	pdata = sensor->host_priv;
+	buscfg = sensor->host_priv;
 
-	ret = ccp2_phyif_config(ccp2, &pdata->bus.ccp2);
+	ret = ccp2_phyif_config(ccp2, &buscfg->bus.ccp2);
 	if (ret < 0)
 		return ret;
 
-	ccp2_vp_config(ccp2, pdata->bus.ccp2.vpclk_div + 1);
+	ccp2_vp_config(ccp2, buscfg->bus.ccp2.vpclk_div + 1);
 
 	v4l2_subdev_call(sensor, sensor, g_skip_top_lines, &lines);
 
 	format = &ccp2->formats[CCP2_PAD_SINK];
 
 	ccp2->if_cfg.data_start = lines;
-	ccp2->if_cfg.crc = pdata->bus.ccp2.crc;
+	ccp2->if_cfg.crc = buscfg->bus.ccp2.crc;
 	ccp2->if_cfg.format = format->code;
 	ccp2->if_cfg.data_size = format->height;
 
diff --git a/drivers/media/platform/omap3isp/ispcsi2.c b/drivers/media/platform/omap3isp/ispcsi2.c
index 09c686d..14d279d 100644
--- a/drivers/media/platform/omap3isp/ispcsi2.c
+++ b/drivers/media/platform/omap3isp/ispcsi2.c
@@ -548,7 +548,7 @@ int omap3isp_csi2_reset(struct isp_csi2_device *csi2)
 
 static int csi2_configure(struct isp_csi2_device *csi2)
 {
-	const struct isp_v4l2_subdevs_group *pdata;
+	const struct isp_bus_cfg *buscfg;
 	struct isp_device *isp = csi2->isp;
 	struct isp_csi2_timing_cfg *timing = &csi2->timing[0];
 	struct v4l2_subdev *sensor;
@@ -565,14 +565,14 @@ static int csi2_configure(struct isp_csi2_device *csi2)
 
 	pad = media_entity_remote_pad(&csi2->pads[CSI2_PAD_SINK]);
 	sensor = media_entity_to_v4l2_subdev(pad->entity);
-	pdata = sensor->host_priv;
+	buscfg = sensor->host_priv;
 
 	csi2->frame_skip = 0;
 	v4l2_subdev_call(sensor, sensor, g_skip_frames, &csi2->frame_skip);
 
-	csi2->ctrl.vp_out_ctrl = pdata->bus.csi2.vpclk_div;
+	csi2->ctrl.vp_out_ctrl = buscfg->bus.csi2.vpclk_div;
 	csi2->ctrl.frame_mode = ISP_CSI2_FRAME_IMMEDIATE;
-	csi2->ctrl.ecc_enable = pdata->bus.csi2.crc;
+	csi2->ctrl.ecc_enable = buscfg->bus.csi2.crc;
 
 	timing->ionum = 1;
 	timing->force_rx_mode = 1;
diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c b/drivers/media/platform/omap3isp/ispcsiphy.c
index e033f22..4486e9f 100644
--- a/drivers/media/platform/omap3isp/ispcsiphy.c
+++ b/drivers/media/platform/omap3isp/ispcsiphy.c
@@ -168,18 +168,18 @@ static int omap3isp_csiphy_config(struct isp_csiphy *phy)
 {
 	struct isp_csi2_device *csi2 = phy->csi2;
 	struct isp_pipeline *pipe = to_isp_pipeline(&csi2->subdev.entity);
-	struct isp_v4l2_subdevs_group *subdevs = pipe->external->host_priv;
+	struct isp_bus_cfg *buscfg = pipe->external->host_priv;
 	struct isp_csiphy_lanes_cfg *lanes;
 	int csi2_ddrclk_khz;
 	unsigned int used_lanes = 0;
 	unsigned int i;
 	u32 reg;
 
-	if (subdevs->interface == ISP_INTERFACE_CCP2B_PHY1
-	    || subdevs->interface == ISP_INTERFACE_CCP2B_PHY2)
-		lanes = &subdevs->bus.ccp2.lanecfg;
+	if (buscfg->interface == ISP_INTERFACE_CCP2B_PHY1
+	    || buscfg->interface == ISP_INTERFACE_CCP2B_PHY2)
+		lanes = &buscfg->bus.ccp2.lanecfg;
 	else
-		lanes = &subdevs->bus.csi2.lanecfg;
+		lanes = &buscfg->bus.csi2.lanecfg;
 
 	/* Clock and data lanes verification */
 	for (i = 0; i < phy->num_data_lanes; i++) {
@@ -203,8 +203,8 @@ static int omap3isp_csiphy_config(struct isp_csiphy *phy)
 	 * issue since the MPU power domain is forced on whilst the
 	 * ISP is in use.
 	 */
-	csiphy_routing_cfg(phy, subdevs->interface, true,
-			   subdevs->bus.ccp2.phy_layer);
+	csiphy_routing_cfg(phy, buscfg->interface, true,
+			   buscfg->bus.ccp2.phy_layer);
 
 	/* DPHY timing configuration */
 	/* CSI-2 is DDR and we only count used lanes. */
@@ -302,11 +302,10 @@ void omap3isp_csiphy_release(struct isp_csiphy *phy)
 		struct isp_csi2_device *csi2 = phy->csi2;
 		struct isp_pipeline *pipe =
 			to_isp_pipeline(&csi2->subdev.entity);
-		struct isp_v4l2_subdevs_group *subdevs =
-			pipe->external->host_priv;
+		struct isp_bus_cfg *buscfg = pipe->external->host_priv;
 
-		csiphy_routing_cfg(phy, subdevs->interface, false,
-				   subdevs->bus.ccp2.phy_layer);
+		csiphy_routing_cfg(phy, buscfg->interface, false,
+				   buscfg->bus.ccp2.phy_layer);
 		csiphy_power_autoswitch_enable(phy, false);
 		csiphy_set_power(phy, ISPCSI2_PHY_CFG_PWR_CMD_OFF);
 		regulator_disable(phy->vdd);
diff --git a/include/media/omap3isp.h b/include/media/omap3isp.h
index 398279d..39e0748 100644
--- a/include/media/omap3isp.h
+++ b/include/media/omap3isp.h
@@ -45,7 +45,7 @@ enum {
 };
 
 /**
- * struct isp_parallel_platform_data - Parallel interface platform data
+ * struct isp_parallel_cfg - Parallel interface configuration
  * @data_lane_shift: Data lane shifter
  *		ISP_LANE_SHIFT_0 - CAMEXT[13:0] -> CAM[13:0]
  *		ISP_LANE_SHIFT_2 - CAMEXT[13:2] -> CAM[11:0]
@@ -62,7 +62,7 @@ enum {
  * @data_pol: Data polarity
  *		0 - Normal, 1 - One's complement
  */
-struct isp_parallel_platform_data {
+struct isp_parallel_cfg {
 	unsigned int data_lane_shift:2;
 	unsigned int clk_pol:1;
 	unsigned int hs_pol:1;
@@ -105,7 +105,7 @@ struct isp_csiphy_lanes_cfg {
 };
 
 /**
- * struct isp_ccp2_platform_data - CCP2 interface platform data
+ * struct isp_ccp2_cfg - CCP2 interface configuration
  * @strobe_clk_pol: Strobe/clock polarity
  *		0 - Non Inverted, 1 - Inverted
  * @crc: Enable the cyclic redundancy check
@@ -117,7 +117,7 @@ struct isp_csiphy_lanes_cfg {
  *		ISP_CCP2_PHY_DATA_STROBE - Data/strobe physical layer
  * @vpclk_div: Video port output clock control
  */
-struct isp_ccp2_platform_data {
+struct isp_ccp2_cfg {
 	unsigned int strobe_clk_pol:1;
 	unsigned int crc:1;
 	unsigned int ccp2_mode:1;
@@ -127,31 +127,31 @@ struct isp_ccp2_platform_data {
 };
 
 /**
- * struct isp_csi2_platform_data - CSI2 interface platform data
+ * struct isp_csi2_cfg - CSI2 interface configuration
  * @crc: Enable the cyclic redundancy check
  * @vpclk_div: Video port output clock control
  */
-struct isp_csi2_platform_data {
+struct isp_csi2_cfg {
 	unsigned crc:1;
 	unsigned vpclk_div:2;
 	struct isp_csiphy_lanes_cfg lanecfg;
 };
 
-struct isp_subdev_i2c_board_info {
-	struct i2c_board_info *board_info;
-	int i2c_adapter_id;
-};
-
-struct isp_v4l2_subdevs_group {
-	struct isp_subdev_i2c_board_info *subdevs;
+struct isp_bus_cfg {
 	enum isp_interface_type interface;
 	union {
-		struct isp_parallel_platform_data parallel;
-		struct isp_ccp2_platform_data ccp2;
-		struct isp_csi2_platform_data csi2;
+		struct isp_parallel_cfg parallel;
+		struct isp_ccp2_cfg ccp2;
+		struct isp_csi2_cfg csi2;
 	} bus; /* gcc < 4.6.0 chokes on anonymous union initializers */
 };
 
+struct isp_platform_subdev {
+	struct i2c_board_info *board_info;
+	int i2c_adapter_id;
+	struct isp_bus_cfg *bus;
+};
+
 struct isp_platform_xclk {
 	const char *dev_id;
 	const char *con_id;
@@ -159,7 +159,7 @@ struct isp_platform_xclk {
 
 struct isp_platform_data {
 	struct isp_platform_xclk xclks[2];
-	struct isp_v4l2_subdevs_group *subdevs;
+	struct isp_platform_subdev *subdevs;
 	void (*set_constraints)(struct isp_device *isp, bool enable);
 };
 
-- 
1.7.10.4

Re: [PATCH 06/15] omap3isp: Refactor device configuration structs for Device Tree

[Igor Grinberg]

On 03/16/15 02:26, Sakari Ailus wrote:
> Make omap3isp configuration data structures more suitable for consumption by
> the DT by separating the I2C bus information of all the sub-devices in a
> group and the ISP bus information from each other. The ISP bus information
> is made a pointer instead of being directly embedded in the struct.
> 
> In the case of the DT only the sensor specific information on the ISP bus
> configuration is retained. The structs are renamed to reflect that.
> 
> After this change the structs needed to describe device configuration can be
> allocated and accessed separately without those needed only in the case of
> platform data. The platform data related structs can be later removed once
> the support for platform data can be removed.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Igor Grinberg <grinberg@compulab.co.il>

For cm-t35 stuff:

Acked-by: Igor Grinberg <grinberg@compulab.co.il>

-- 
Regards,
Igor.

[PATCH 07/15] omap3isp: Rename regulators to better suit the Device Tree

[Sakari Ailus]

Rename VDD_CSIPHY1 as vdd-csiphy1 and VDD_CSIPHY2 as vdd-csiphy2.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/omap3isp/isp.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 1b5c6df..c045318 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2292,8 +2292,8 @@ static int isp_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, isp);
 
 	/* Regulators */
-	isp->isp_csiphy1.vdd = devm_regulator_get(&pdev->dev, "VDD_CSIPHY1");
-	isp->isp_csiphy2.vdd = devm_regulator_get(&pdev->dev, "VDD_CSIPHY2");
+	isp->isp_csiphy1.vdd = devm_regulator_get(&pdev->dev, "vdd-csiphy1");
+	isp->isp_csiphy2.vdd = devm_regulator_get(&pdev->dev, "vdd-csiphy2");
 
 	/* Clocks
 	 *
-- 
1.7.10.4

[PATCH 08/15] omap3isp: Calculate vpclk_div for CSI-2

[Sakari Ailus]

The video port clock is l3_ick divided by vpclk_div. This clock must be high
enough for the external pixel rate. The video port requires two clock cycles
to process a pixel.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/omap3isp/ispcsi2.c |    8 +++++++-
 include/media/omap3isp.h                  |    2 --
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/omap3isp/ispcsi2.c b/drivers/media/platform/omap3isp/ispcsi2.c
index 14d279d..97cdfeb 100644
--- a/drivers/media/platform/omap3isp/ispcsi2.c
+++ b/drivers/media/platform/omap3isp/ispcsi2.c
@@ -548,6 +548,7 @@ int omap3isp_csi2_reset(struct isp_csi2_device *csi2)
 
 static int csi2_configure(struct isp_csi2_device *csi2)
 {
+	struct isp_pipeline *pipe = to_isp_pipeline(&csi2->subdev.entity);
 	const struct isp_bus_cfg *buscfg;
 	struct isp_device *isp = csi2->isp;
 	struct isp_csi2_timing_cfg *timing = &csi2->timing[0];
@@ -570,7 +571,12 @@ static int csi2_configure(struct isp_csi2_device *csi2)
 	csi2->frame_skip = 0;
 	v4l2_subdev_call(sensor, sensor, g_skip_frames, &csi2->frame_skip);
 
-	csi2->ctrl.vp_out_ctrl = buscfg->bus.csi2.vpclk_div;
+	csi2->ctrl.vp_out_ctrl =
+		clamp_t(unsigned int, pipe->l3_ick / pipe->external_rate - 1,
+			1, 3);
+	dev_dbg(isp->dev, "%s: l3_ick %lu, external_rate %u, vp_out_ctrl %u\n",
+		__func__, pipe->l3_ick,  pipe->external_rate,
+		csi2->ctrl.vp_out_ctrl);
 	csi2->ctrl.frame_mode = ISP_CSI2_FRAME_IMMEDIATE;
 	csi2->ctrl.ecc_enable = buscfg->bus.csi2.crc;
 
diff --git a/include/media/omap3isp.h b/include/media/omap3isp.h
index 39e0748..0f0c08b 100644
--- a/include/media/omap3isp.h
+++ b/include/media/omap3isp.h
@@ -129,11 +129,9 @@ struct isp_ccp2_cfg {
 /**
  * struct isp_csi2_cfg - CSI2 interface configuration
  * @crc: Enable the cyclic redundancy check
- * @vpclk_div: Video port output clock control
  */
 struct isp_csi2_cfg {
 	unsigned crc:1;
-	unsigned vpclk_div:2;
 	struct isp_csiphy_lanes_cfg lanecfg;
 };
 
-- 
1.7.10.4

[PATCH 09/15] omap3isp: Replace mmio_base_phys array with the histogram block base

[Sakari Ailus]

Only the histogram sub-block driver uses the physical address. Do not store
it for other sub-blocks.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/platform/omap3isp/isp.c     |    3 ++-
 drivers/media/platform/omap3isp/isp.h     |    6 +++---
 drivers/media/platform/omap3isp/isphist.c |    3 +--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index c045318..68d7edfc 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2247,7 +2247,8 @@ static int isp_map_mem_resource(struct platform_device *pdev,
 	if (IS_ERR(isp->mmio_base[res]))
 		return PTR_ERR(isp->mmio_base[res]);
 
-	isp->mmio_base_phys[res] = mem->start;
+	if (res == OMAP3_ISP_IOMEM_HIST)
+		isp->mmio_hist_base_phys = mem->start;
 
 	return 0;
 }
diff --git a/drivers/media/platform/omap3isp/isp.h b/drivers/media/platform/omap3isp/isp.h
index b932a6f..9535524 100644
--- a/drivers/media/platform/omap3isp/isp.h
+++ b/drivers/media/platform/omap3isp/isp.h
@@ -138,8 +138,8 @@ struct isp_xclk {
  * @irq_num: Currently used IRQ number.
  * @mmio_base: Array with kernel base addresses for ioremapped ISP register
  *             regions.
- * @mmio_base_phys: Array with physical L4 bus addresses for ISP register
- *                  regions.
+ * @mmio_hist_base_phys: Physical L4 bus address for ISP hist block register
+ *			 region.
  * @mapping: IOMMU mapping
  * @stat_lock: Spinlock for handling statistics
  * @isp_mutex: Mutex for serializing requests to ISP.
@@ -175,7 +175,7 @@ struct isp_device {
 	unsigned int irq_num;
 
 	void __iomem *mmio_base[OMAP3_ISP_IOMEM_LAST];
-	unsigned long mmio_base_phys[OMAP3_ISP_IOMEM_LAST];
+	unsigned long mmio_hist_base_phys;
 
 	struct dma_iommu_mapping *mapping;
 
diff --git a/drivers/media/platform/omap3isp/isphist.c b/drivers/media/platform/omap3isp/isphist.c
index 738b946..7138b04 100644
--- a/drivers/media/platform/omap3isp/isphist.c
+++ b/drivers/media/platform/omap3isp/isphist.c
@@ -193,8 +193,7 @@ static int hist_buf_dma(struct ispstat *hist)
 	omap3isp_flush(hist->isp);
 
 	memset(&cfg, 0, sizeof(cfg));
-	cfg.src_addr = hist->isp->mmio_base_phys[OMAP3_ISP_IOMEM_HIST]
-		     + ISPHIST_DATA;
+	cfg.src_addr = hist->isp->mmio_hist_base_phys + ISPHIST_DATA;
 	cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
 	cfg.src_maxburst = hist->buf_size / 4;
 
-- 
1.7.10.4

[PATCH 10/15] omap3isp: Move the syscon register out of the ISP register maps

[Sakari Ailus]

The syscon register isn't part of the ISP, use it through the syscom driver
regmap instead. The syscom block is considered to be from 343x on ISP
revision 2.0 whereas 15.0 is assumed to have 3630 syscon.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/devices.c               |   10 ----------
 drivers/media/platform/omap3isp/isp.c       |   20 ++++++++++++++++----
 drivers/media/platform/omap3isp/isp.h       |   19 +++++++++++++++++--
 drivers/media/platform/omap3isp/ispcsiphy.c |   20 +++++++++-----------
 4 files changed, 42 insertions(+), 27 deletions(-)

diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index 1afb50d..e945957 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -143,16 +143,6 @@ static struct resource omap3isp_resources[] = {
 		.flags		= IORESOURCE_MEM,
 	},
 	{
-		.start		= OMAP343X_CTRL_BASE + OMAP343X_CONTROL_CSIRXFE,
-		.end		= OMAP343X_CTRL_BASE + OMAP343X_CONTROL_CSIRXFE + 3,
-		.flags		= IORESOURCE_MEM,
-	},
-	{
-		.start		= OMAP343X_CTRL_BASE + OMAP3630_CONTROL_CAMERA_PHY_CTRL,
-		.end		= OMAP343X_CTRL_BASE + OMAP3630_CONTROL_CAMERA_PHY_CTRL + 3,
-		.flags		= IORESOURCE_MEM,
-	},
-	{
 		.start		= 24 + OMAP_INTC_START,
 		.flags		= IORESOURCE_IRQ,
 	}
diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 68d7edfc..83b4368 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -51,6 +51,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
+#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/omap-iommu.h>
 #include <linux/platform_device.h>
@@ -94,8 +95,9 @@ static const struct isp_res_mapping isp_res_maps[] = {
 		       1 << OMAP3_ISP_IOMEM_RESZ |
 		       1 << OMAP3_ISP_IOMEM_SBL |
 		       1 << OMAP3_ISP_IOMEM_CSI2A_REGS1 |
-		       1 << OMAP3_ISP_IOMEM_CSIPHY2 |
-		       1 << OMAP3_ISP_IOMEM_343X_CONTROL_CSIRXFE,
+		       1 << OMAP3_ISP_IOMEM_CSIPHY2,
+		.syscon_offset = 0xdc,
+		.phy_type = ISP_PHY_TYPE_3430,
 	},
 	{
 		.isp_rev = ISP_REVISION_15_0,
@@ -112,8 +114,9 @@ static const struct isp_res_mapping isp_res_maps[] = {
 		       1 << OMAP3_ISP_IOMEM_CSI2A_REGS2 |
 		       1 << OMAP3_ISP_IOMEM_CSI2C_REGS1 |
 		       1 << OMAP3_ISP_IOMEM_CSIPHY1 |
-		       1 << OMAP3_ISP_IOMEM_CSI2C_REGS2 |
-		       1 << OMAP3_ISP_IOMEM_3630_CONTROL_CAMERA_PHY_CTRL,
+		       1 << OMAP3_ISP_IOMEM_CSI2C_REGS2,
+		.syscon_offset = 0x2f0,
+		.phy_type = ISP_PHY_TYPE_3630,
 	},
 };
 
@@ -2352,6 +2355,15 @@ static int isp_probe(struct platform_device *pdev)
 		}
 	}
 
+	isp->syscon = syscon_regmap_lookup_by_pdevname("syscon.0");
+	if (IS_ERR(isp->syscon)) {
+		ret = PTR_ERR(isp->syscon);
+		goto error_isp;
+	}
+
+	isp->syscon_offset = isp_res_maps[m].syscon_offset;
+	isp->phy_type = isp_res_maps[m].phy_type;
+
 	/* IOMMU */
 	ret = isp_attach_iommu(isp);
 	if (ret < 0) {
diff --git a/drivers/media/platform/omap3isp/isp.h b/drivers/media/platform/omap3isp/isp.h
index 9535524..03d2129 100644
--- a/drivers/media/platform/omap3isp/isp.h
+++ b/drivers/media/platform/omap3isp/isp.h
@@ -59,8 +59,6 @@ enum isp_mem_resources {
 	OMAP3_ISP_IOMEM_CSI2C_REGS1,
 	OMAP3_ISP_IOMEM_CSIPHY1,
 	OMAP3_ISP_IOMEM_CSI2C_REGS2,
-	OMAP3_ISP_IOMEM_343X_CONTROL_CSIRXFE,
-	OMAP3_ISP_IOMEM_3630_CONTROL_CAMERA_PHY_CTRL,
 	OMAP3_ISP_IOMEM_LAST
 };
 
@@ -93,14 +91,25 @@ enum isp_subclk_resource {
 /* ISP2P: OMAP 36xx */
 #define ISP_REVISION_15_0		0xF0
 
+#define ISP_PHY_TYPE_3430		0
+#define ISP_PHY_TYPE_3630		1
+
+struct regmap;
+
 /*
  * struct isp_res_mapping - Map ISP io resources to ISP revision.
  * @isp_rev: ISP_REVISION_x_x
  * @map: bitmap for enum isp_mem_resources
+ * @syscon_offset: offset of the syscon register for 343x / 3630
+ *	    (CONTROL_CSIRXFE / CONTROL_CAMERA_PHY_CTRL, respectively)
+ *	    from the syscon base address
+ * @phy_type: ISP_PHY_TYPE_{3430,3630}
  */
 struct isp_res_mapping {
 	u32 isp_rev;
 	u32 map;
+	u32 syscon_offset;
+	u32 phy_type;
 };
 
 /*
@@ -140,6 +149,9 @@ struct isp_xclk {
  *             regions.
  * @mmio_hist_base_phys: Physical L4 bus address for ISP hist block register
  *			 region.
+ * @syscon: Regmap for the syscon register space
+ * @syscon_offset: Offset of the CSIPHY control register in syscon
+ * @phy_type: ISP_PHY_TYPE_{3430,3630}
  * @mapping: IOMMU mapping
  * @stat_lock: Spinlock for handling statistics
  * @isp_mutex: Mutex for serializing requests to ISP.
@@ -176,6 +188,9 @@ struct isp_device {
 
 	void __iomem *mmio_base[OMAP3_ISP_IOMEM_LAST];
 	unsigned long mmio_hist_base_phys;
+	struct regmap *syscon;
+	u32 syscon_offset;
+	u32 phy_type;
 
 	struct dma_iommu_mapping *mapping;
 
diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c b/drivers/media/platform/omap3isp/ispcsiphy.c
index 4486e9f..d91dde1 100644
--- a/drivers/media/platform/omap3isp/ispcsiphy.c
+++ b/drivers/media/platform/omap3isp/ispcsiphy.c
@@ -16,6 +16,7 @@
 
 #include <linux/delay.h>
 #include <linux/device.h>
+#include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 
 #include "isp.h"
@@ -26,10 +27,11 @@ static void csiphy_routing_cfg_3630(struct isp_csiphy *phy,
 				    enum isp_interface_type iface,
 				    bool ccp2_strobe)
 {
-	u32 reg = isp_reg_readl(
-		phy->isp, OMAP3_ISP_IOMEM_3630_CONTROL_CAMERA_PHY_CTRL, 0);
+	u32 reg;
 	u32 shift, mode;
 
+	regmap_read(phy->isp->syscon, phy->isp->syscon_offset, &reg);
+
 	switch (iface) {
 	default:
 	/* Should not happen in practice, but let's keep the compiler happy. */
@@ -63,8 +65,7 @@ static void csiphy_routing_cfg_3630(struct isp_csiphy *phy,
 	reg &= ~(OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_MASK << shift);
 	reg |= mode << shift;
 
-	isp_reg_writel(phy->isp, reg,
-		       OMAP3_ISP_IOMEM_3630_CONTROL_CAMERA_PHY_CTRL, 0);
+	regmap_write(phy->isp->syscon, phy->isp->syscon_offset, reg);
 }
 
 static void csiphy_routing_cfg_3430(struct isp_csiphy *phy, u32 iface, bool on,
@@ -78,16 +79,14 @@ static void csiphy_routing_cfg_3430(struct isp_csiphy *phy, u32 iface, bool on,
 		return;
 
 	if (!on) {
-		isp_reg_writel(phy->isp, 0,
-			       OMAP3_ISP_IOMEM_343X_CONTROL_CSIRXFE, 0);
+		regmap_write(phy->isp->syscon, phy->isp->syscon_offset, 0);
 		return;
 	}
 
 	if (ccp2_strobe)
 		csirxfe |= OMAP343X_CONTROL_CSIRXFE_SELFORM;
 
-	isp_reg_writel(phy->isp, csirxfe,
-		       OMAP3_ISP_IOMEM_343X_CONTROL_CSIRXFE, 0);
+	regmap_write(phy->isp->syscon, phy->isp->syscon_offset, csirxfe);
 }
 
 /*
@@ -106,10 +105,9 @@ static void csiphy_routing_cfg(struct isp_csiphy *phy,
 			       enum isp_interface_type iface, bool on,
 			       bool ccp2_strobe)
 {
-	if (phy->isp->mmio_base[OMAP3_ISP_IOMEM_3630_CONTROL_CAMERA_PHY_CTRL]
-	    && on)
+	if (phy->isp->phy_type == ISP_PHY_TYPE_3630 && on)
 		return csiphy_routing_cfg_3630(phy, iface, ccp2_strobe);
-	if (phy->isp->mmio_base[OMAP3_ISP_IOMEM_343X_CONTROL_CSIRXFE])
+	if (phy->isp->phy_type == ISP_PHY_TYPE_3430)
 		return csiphy_routing_cfg_3430(phy, iface, on, ccp2_strobe);
 }
 
-- 
1.7.10.4

[PATCH v1.1 10/15] omap3isp: Move the syscon register out of the ISP register maps

[Sakari Ailus]

The syscon register isn't part of the ISP, use it through the syscom driver
regmap instead. The syscom block is considered to be from 343x on ISP
revision 2.0 whereas 15.0 is assumed to have 3630 syscon.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Tony Lindgren <tony@atomide.com>
---
since v1:

- Depend on MFD_SYSCON in Kconfig

 arch/arm/mach-omap2/devices.c               |   10 ----------
 drivers/media/platform/Kconfig              |    1 +
 drivers/media/platform/omap3isp/isp.c       |   20 ++++++++++++++++----
 drivers/media/platform/omap3isp/isp.h       |   19 +++++++++++++++++--
 drivers/media/platform/omap3isp/ispcsiphy.c |   20 +++++++++-----------
 5 files changed, 43 insertions(+), 27 deletions(-)

diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index 1afb50d..e945957 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -143,16 +143,6 @@ static struct resource omap3isp_resources[] = {
 		.flags		= IORESOURCE_MEM,
 	},
 	{
-		.start		= OMAP343X_CTRL_BASE + OMAP343X_CONTROL_CSIRXFE,
-		.end		= OMAP343X_CTRL_BASE + OMAP343X_CONTROL_CSIRXFE + 3,
-		.flags		= IORESOURCE_MEM,
-	},
-	{
-		.start		= OMAP343X_CTRL_BASE + OMAP3630_CONTROL_CAMERA_PHY_CTRL,
-		.end		= OMAP343X_CTRL_BASE + OMAP3630_CONTROL_CAMERA_PHY_CTRL + 3,
-		.flags		= IORESOURCE_MEM,
-	},
-	{
 		.start		= 24 + OMAP_INTC_START,
 		.flags		= IORESOURCE_IRQ,
 	}
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 2e30be5..272dc8c 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -90,6 +90,7 @@ config VIDEO_OMAP3
 	select ARM_DMA_USE_IOMMU
 	select OMAP_IOMMU
 	select VIDEOBUF2_DMA_CONTIG
+	select MFD_SYSCON
 	---help---
 	  Driver for an OMAP 3 camera controller.
 
diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 68d7edfc..83b4368 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -51,6 +51,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
+#include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/omap-iommu.h>
 #include <linux/platform_device.h>
@@ -94,8 +95,9 @@ static const struct isp_res_mapping isp_res_maps[] = {
 		       1 << OMAP3_ISP_IOMEM_RESZ |
 		       1 << OMAP3_ISP_IOMEM_SBL |
 		       1 << OMAP3_ISP_IOMEM_CSI2A_REGS1 |
-		       1 << OMAP3_ISP_IOMEM_CSIPHY2 |
-		       1 << OMAP3_ISP_IOMEM_343X_CONTROL_CSIRXFE,
+		       1 << OMAP3_ISP_IOMEM_CSIPHY2,
+		.syscon_offset = 0xdc,
+		.phy_type = ISP_PHY_TYPE_3430,
 	},
 	{
 		.isp_rev = ISP_REVISION_15_0,
@@ -112,8 +114,9 @@ static const struct isp_res_mapping isp_res_maps[] = {
 		       1 << OMAP3_ISP_IOMEM_CSI2A_REGS2 |
 		       1 << OMAP3_ISP_IOMEM_CSI2C_REGS1 |
 		       1 << OMAP3_ISP_IOMEM_CSIPHY1 |
-		       1 << OMAP3_ISP_IOMEM_CSI2C_REGS2 |
-		       1 << OMAP3_ISP_IOMEM_3630_CONTROL_CAMERA_PHY_CTRL,
+		       1 << OMAP3_ISP_IOMEM_CSI2C_REGS2,
+		.syscon_offset = 0x2f0,
+		.phy_type = ISP_PHY_TYPE_3630,
 	},
 };
 
@@ -2352,6 +2355,15 @@ static int isp_probe(struct platform_device *pdev)
 		}
 	}
 
+	isp->syscon = syscon_regmap_lookup_by_pdevname("syscon.0");
+	if (IS_ERR(isp->syscon)) {
+		ret = PTR_ERR(isp->syscon);
+		goto error_isp;
+	}
+
+	isp->syscon_offset = isp_res_maps[m].syscon_offset;
+	isp->phy_type = isp_res_maps[m].phy_type;
+
 	/* IOMMU */
 	ret = isp_attach_iommu(isp);
 	if (ret < 0) {
diff --git a/drivers/media/platform/omap3isp/isp.h b/drivers/media/platform/omap3isp/isp.h
index 9535524..03d2129 100644
--- a/drivers/media/platform/omap3isp/isp.h
+++ b/drivers/media/platform/omap3isp/isp.h
@@ -59,8 +59,6 @@ enum isp_mem_resources {
 	OMAP3_ISP_IOMEM_CSI2C_REGS1,
 	OMAP3_ISP_IOMEM_CSIPHY1,
 	OMAP3_ISP_IOMEM_CSI2C_REGS2,
-	OMAP3_ISP_IOMEM_343X_CONTROL_CSIRXFE,
-	OMAP3_ISP_IOMEM_3630_CONTROL_CAMERA_PHY_CTRL,
 	OMAP3_ISP_IOMEM_LAST
 };
 
@@ -93,14 +91,25 @@ enum isp_subclk_resource {
 /* ISP2P: OMAP 36xx */
 #define ISP_REVISION_15_0		0xF0
 
+#define ISP_PHY_TYPE_3430		0
+#define ISP_PHY_TYPE_3630		1
+
+struct regmap;
+
 /*
  * struct isp_res_mapping - Map ISP io resources to ISP revision.
  * @isp_rev: ISP_REVISION_x_x
  * @map: bitmap for enum isp_mem_resources
+ * @syscon_offset: offset of the syscon register for 343x / 3630
+ *	    (CONTROL_CSIRXFE / CONTROL_CAMERA_PHY_CTRL, respectively)
+ *	    from the syscon base address
+ * @phy_type: ISP_PHY_TYPE_{3430,3630}
  */
 struct isp_res_mapping {
 	u32 isp_rev;
 	u32 map;
+	u32 syscon_offset;
+	u32 phy_type;
 };
 
 /*
@@ -140,6 +149,9 @@ struct isp_xclk {
  *             regions.
  * @mmio_hist_base_phys: Physical L4 bus address for ISP hist block register
  *			 region.
+ * @syscon: Regmap for the syscon register space
+ * @syscon_offset: Offset of the CSIPHY control register in syscon
+ * @phy_type: ISP_PHY_TYPE_{3430,3630}
  * @mapping: IOMMU mapping
  * @stat_lock: Spinlock for handling statistics
  * @isp_mutex: Mutex for serializing requests to ISP.
@@ -176,6 +188,9 @@ struct isp_device {
 
 	void __iomem *mmio_base[OMAP3_ISP_IOMEM_LAST];
 	unsigned long mmio_hist_base_phys;
+	struct regmap *syscon;
+	u32 syscon_offset;
+	u32 phy_type;
 
 	struct dma_iommu_mapping *mapping;
 
diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c b/drivers/media/platform/omap3isp/ispcsiphy.c
index 4486e9f..d91dde1 100644
--- a/drivers/media/platform/omap3isp/ispcsiphy.c
+++ b/drivers/media/platform/omap3isp/ispcsiphy.c
@@ -16,6 +16,7 @@
 
 #include <linux/delay.h>
 #include <linux/device.h>
+#include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 
 #include "isp.h"
@@ -26,10 +27,11 @@ static void csiphy_routing_cfg_3630(struct isp_csiphy *phy,
 				    enum isp_interface_type iface,
 				    bool ccp2_strobe)
 {
-	u32 reg = isp_reg_readl(
-		phy->isp, OMAP3_ISP_IOMEM_3630_CONTROL_CAMERA_PHY_CTRL, 0);
+	u32 reg;
 	u32 shift, mode;
 
+	regmap_read(phy->isp->syscon, phy->isp->syscon_offset, &reg);
+
 	switch (iface) {
 	default:
 	/* Should not happen in practice, but let's keep the compiler happy. */
@@ -63,8 +65,7 @@ static void csiphy_routing_cfg_3630(struct isp_csiphy *phy,
 	reg &= ~(OMAP3630_CONTROL_CAMERA_PHY_CTRL_CAMMODE_MASK << shift);
 	reg |= mode << shift;
 
-	isp_reg_writel(phy->isp, reg,
-		       OMAP3_ISP_IOMEM_3630_CONTROL_CAMERA_PHY_CTRL, 0);
+	regmap_write(phy->isp->syscon, phy->isp->syscon_offset, reg);
 }
 
 static void csiphy_routing_cfg_3430(struct isp_csiphy *phy, u32 iface, bool on,
@@ -78,16 +79,14 @@ static void csiphy_routing_cfg_3430(struct isp_csiphy *phy, u32 iface, bool on,
 		return;
 
 	if (!on) {
-		isp_reg_writel(phy->isp, 0,
-			       OMAP3_ISP_IOMEM_343X_CONTROL_CSIRXFE, 0);
+		regmap_write(phy->isp->syscon, phy->isp->syscon_offset, 0);
 		return;
 	}
 
 	if (ccp2_strobe)
 		csirxfe |= OMAP343X_CONTROL_CSIRXFE_SELFORM;
 
-	isp_reg_writel(phy->isp, csirxfe,
-		       OMAP3_ISP_IOMEM_343X_CONTROL_CSIRXFE, 0);
+	regmap_write(phy->isp->syscon, phy->isp->syscon_offset, csirxfe);
 }
 
 /*
@@ -106,10 +105,9 @@ static void csiphy_routing_cfg(struct isp_csiphy *phy,
 			       enum isp_interface_type iface, bool on,
 			       bool ccp2_strobe)
 {
-	if (phy->isp->mmio_base[OMAP3_ISP_IOMEM_3630_CONTROL_CAMERA_PHY_CTRL]
-	    && on)
+	if (phy->isp->phy_type == ISP_PHY_TYPE_3630 && on)
 		return csiphy_routing_cfg_3630(phy, iface, ccp2_strobe);
-	if (phy->isp->mmio_base[OMAP3_ISP_IOMEM_343X_CONTROL_CSIRXFE])
+	if (phy->isp->phy_type == ISP_PHY_TYPE_3430)
 		return csiphy_routing_cfg_3430(phy, iface, on, ccp2_strobe);
 }
 
-- 
1.7.10.4

[PATCH 11/15] omap3isp: Replace many MMIO regions by two

[Sakari Ailus]

The omap3isp MMIO register block is contiguous in the MMIO register space
apart from the fact that the ISP IOMMU register block is in the middle of
the area. Ioremap it at two occasions, and keep the rest of the layout of
the register space internal to the omap3isp driver.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/mach-omap2/devices.c         |   66 +------------------
 arch/arm/mach-omap2/omap34xx.h        |   36 +----------
 drivers/media/platform/omap3isp/isp.c |  113 +++++++++++++++++----------------
 drivers/media/platform/omap3isp/isp.h |    4 +-
 4 files changed, 66 insertions(+), 153 deletions(-)

diff --git a/arch/arm/mach-omap2/devices.c b/arch/arm/mach-omap2/devices.c
index e945957..990338f 100644
--- a/arch/arm/mach-omap2/devices.c
+++ b/arch/arm/mach-omap2/devices.c
@@ -74,72 +74,12 @@ omap_postcore_initcall(omap3_l3_init);
 static struct resource omap3isp_resources[] = {
 	{
 		.start		= OMAP3430_ISP_BASE,
-		.end		= OMAP3430_ISP_END,
+		.end		= OMAP3430_ISP_BASE + 0x12fc,
 		.flags		= IORESOURCE_MEM,
 	},
 	{
-		.start		= OMAP3430_ISP_CCP2_BASE,
-		.end		= OMAP3430_ISP_CCP2_END,
-		.flags		= IORESOURCE_MEM,
-	},
-	{
-		.start		= OMAP3430_ISP_CCDC_BASE,
-		.end		= OMAP3430_ISP_CCDC_END,
-		.flags		= IORESOURCE_MEM,
-	},
-	{
-		.start		= OMAP3430_ISP_HIST_BASE,
-		.end		= OMAP3430_ISP_HIST_END,
-		.flags		= IORESOURCE_MEM,
-	},
-	{
-		.start		= OMAP3430_ISP_H3A_BASE,
-		.end		= OMAP3430_ISP_H3A_END,
-		.flags		= IORESOURCE_MEM,
-	},
-	{
-		.start		= OMAP3430_ISP_PREV_BASE,
-		.end		= OMAP3430_ISP_PREV_END,
-		.flags		= IORESOURCE_MEM,
-	},
-	{
-		.start		= OMAP3430_ISP_RESZ_BASE,
-		.end		= OMAP3430_ISP_RESZ_END,
-		.flags		= IORESOURCE_MEM,
-	},
-	{
-		.start		= OMAP3430_ISP_SBL_BASE,
-		.end		= OMAP3430_ISP_SBL_END,
-		.flags		= IORESOURCE_MEM,
-	},
-	{
-		.start		= OMAP3430_ISP_CSI2A_REGS1_BASE,
-		.end		= OMAP3430_ISP_CSI2A_REGS1_END,
-		.flags		= IORESOURCE_MEM,
-	},
-	{
-		.start		= OMAP3430_ISP_CSIPHY2_BASE,
-		.end		= OMAP3430_ISP_CSIPHY2_END,
-		.flags		= IORESOURCE_MEM,
-	},
-	{
-		.start		= OMAP3630_ISP_CSI2A_REGS2_BASE,
-		.end		= OMAP3630_ISP_CSI2A_REGS2_END,
-		.flags		= IORESOURCE_MEM,
-	},
-	{
-		.start		= OMAP3630_ISP_CSI2C_REGS1_BASE,
-		.end		= OMAP3630_ISP_CSI2C_REGS1_END,
-		.flags		= IORESOURCE_MEM,
-	},
-	{
-		.start		= OMAP3630_ISP_CSIPHY1_BASE,
-		.end		= OMAP3630_ISP_CSIPHY1_END,
-		.flags		= IORESOURCE_MEM,
-	},
-	{
-		.start		= OMAP3630_ISP_CSI2C_REGS2_BASE,
-		.end		= OMAP3630_ISP_CSI2C_REGS2_END,
+		.start		= OMAP3430_ISP_BASE2,
+		.end		= OMAP3430_ISP_BASE2 + 0x0600,
 		.flags		= IORESOURCE_MEM,
 	},
 	{
diff --git a/arch/arm/mach-omap2/omap34xx.h b/arch/arm/mach-omap2/omap34xx.h
index c0d1b4b..ed0024d 100644
--- a/arch/arm/mach-omap2/omap34xx.h
+++ b/arch/arm/mach-omap2/omap34xx.h
@@ -46,39 +46,9 @@
 
 #define OMAP34XX_IC_BASE	0x48200000
 
-#define OMAP3430_ISP_BASE		(L4_34XX_BASE + 0xBC000)
-#define OMAP3430_ISP_CBUFF_BASE		(OMAP3430_ISP_BASE + 0x0100)
-#define OMAP3430_ISP_CCP2_BASE		(OMAP3430_ISP_BASE + 0x0400)
-#define OMAP3430_ISP_CCDC_BASE		(OMAP3430_ISP_BASE + 0x0600)
-#define OMAP3430_ISP_HIST_BASE		(OMAP3430_ISP_BASE + 0x0A00)
-#define OMAP3430_ISP_H3A_BASE		(OMAP3430_ISP_BASE + 0x0C00)
-#define OMAP3430_ISP_PREV_BASE		(OMAP3430_ISP_BASE + 0x0E00)
-#define OMAP3430_ISP_RESZ_BASE		(OMAP3430_ISP_BASE + 0x1000)
-#define OMAP3430_ISP_SBL_BASE		(OMAP3430_ISP_BASE + 0x1200)
-#define OMAP3430_ISP_MMU_BASE		(OMAP3430_ISP_BASE + 0x1400)
-#define OMAP3430_ISP_CSI2A_REGS1_BASE	(OMAP3430_ISP_BASE + 0x1800)
-#define OMAP3430_ISP_CSIPHY2_BASE	(OMAP3430_ISP_BASE + 0x1970)
-#define OMAP3630_ISP_CSI2A_REGS2_BASE	(OMAP3430_ISP_BASE + 0x19C0)
-#define OMAP3630_ISP_CSI2C_REGS1_BASE	(OMAP3430_ISP_BASE + 0x1C00)
-#define OMAP3630_ISP_CSIPHY1_BASE	(OMAP3430_ISP_BASE + 0x1D70)
-#define OMAP3630_ISP_CSI2C_REGS2_BASE	(OMAP3430_ISP_BASE + 0x1DC0)
-
-#define OMAP3430_ISP_END		(OMAP3430_ISP_BASE         + 0x06F)
-#define OMAP3430_ISP_CBUFF_END		(OMAP3430_ISP_CBUFF_BASE   + 0x077)
-#define OMAP3430_ISP_CCP2_END		(OMAP3430_ISP_CCP2_BASE    + 0x1EF)
-#define OMAP3430_ISP_CCDC_END		(OMAP3430_ISP_CCDC_BASE    + 0x0A7)
-#define OMAP3430_ISP_HIST_END		(OMAP3430_ISP_HIST_BASE    + 0x047)
-#define OMAP3430_ISP_H3A_END		(OMAP3430_ISP_H3A_BASE     + 0x05F)
-#define OMAP3430_ISP_PREV_END		(OMAP3430_ISP_PREV_BASE    + 0x09F)
-#define OMAP3430_ISP_RESZ_END		(OMAP3430_ISP_RESZ_BASE    + 0x0AB)
-#define OMAP3430_ISP_SBL_END		(OMAP3430_ISP_SBL_BASE     + 0x0FB)
-#define OMAP3430_ISP_MMU_END		(OMAP3430_ISP_MMU_BASE     + 0x06F)
-#define OMAP3430_ISP_CSI2A_REGS1_END	(OMAP3430_ISP_CSI2A_REGS1_BASE + 0x16F)
-#define OMAP3430_ISP_CSIPHY2_END	(OMAP3430_ISP_CSIPHY2_BASE + 0x00B)
-#define OMAP3630_ISP_CSI2A_REGS2_END	(OMAP3630_ISP_CSI2A_REGS2_BASE + 0x3F)
-#define OMAP3630_ISP_CSI2C_REGS1_END	(OMAP3630_ISP_CSI2C_REGS1_BASE + 0x16F)
-#define OMAP3630_ISP_CSIPHY1_END	(OMAP3630_ISP_CSIPHY1_BASE + 0x00B)
-#define OMAP3630_ISP_CSI2C_REGS2_END	(OMAP3630_ISP_CSI2C_REGS2_BASE + 0x3F)
+#define OMAP3430_ISP_BASE	(L4_34XX_BASE + 0xBC000)
+#define OMAP3430_ISP_MMU_BASE	(OMAP3430_ISP_BASE + 0x1400)
+#define OMAP3430_ISP_BASE2	(OMAP3430_ISP_BASE + 0x1800)
 
 #define OMAP34XX_HSUSB_OTG_BASE	(L4_34XX_BASE + 0xAB000)
 #define OMAP34XX_USBTLL_BASE	(L4_34XX_BASE + 0x62000)
diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 83b4368..992e74c 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -86,35 +86,43 @@ static void isp_restore_ctx(struct isp_device *isp);
 static const struct isp_res_mapping isp_res_maps[] = {
 	{
 		.isp_rev = ISP_REVISION_2_0,
-		.map = 1 << OMAP3_ISP_IOMEM_MAIN |
-		       1 << OMAP3_ISP_IOMEM_CCP2 |
-		       1 << OMAP3_ISP_IOMEM_CCDC |
-		       1 << OMAP3_ISP_IOMEM_HIST |
-		       1 << OMAP3_ISP_IOMEM_H3A |
-		       1 << OMAP3_ISP_IOMEM_PREV |
-		       1 << OMAP3_ISP_IOMEM_RESZ |
-		       1 << OMAP3_ISP_IOMEM_SBL |
-		       1 << OMAP3_ISP_IOMEM_CSI2A_REGS1 |
-		       1 << OMAP3_ISP_IOMEM_CSIPHY2,
+		.offset = {
+			/* first MMIO area */
+			0x0000, /* base, len 0x0070 */
+			0x0400, /* ccp2, len 0x01f0 */
+			0x0600, /* ccdc, len 0x00a8 */
+			0x0a00, /* hist, len 0x0048 */
+			0x0c00, /* h3a, len 0x0060 */
+			0x0e00, /* preview, len 0x00a0 */
+			0x1000, /* resizer, len 0x00ac */
+			0x1200, /* sbl, len 0x00fc */
+			/* second MMIO area */
+			0x0000, /* csi2a, len 0x0170 */
+			0x0170, /* csiphy2, len 0x000c */
+		},
 		.syscon_offset = 0xdc,
 		.phy_type = ISP_PHY_TYPE_3430,
 	},
 	{
 		.isp_rev = ISP_REVISION_15_0,
-		.map = 1 << OMAP3_ISP_IOMEM_MAIN |
-		       1 << OMAP3_ISP_IOMEM_CCP2 |
-		       1 << OMAP3_ISP_IOMEM_CCDC |
-		       1 << OMAP3_ISP_IOMEM_HIST |
-		       1 << OMAP3_ISP_IOMEM_H3A |
-		       1 << OMAP3_ISP_IOMEM_PREV |
-		       1 << OMAP3_ISP_IOMEM_RESZ |
-		       1 << OMAP3_ISP_IOMEM_SBL |
-		       1 << OMAP3_ISP_IOMEM_CSI2A_REGS1 |
-		       1 << OMAP3_ISP_IOMEM_CSIPHY2 |
-		       1 << OMAP3_ISP_IOMEM_CSI2A_REGS2 |
-		       1 << OMAP3_ISP_IOMEM_CSI2C_REGS1 |
-		       1 << OMAP3_ISP_IOMEM_CSIPHY1 |
-		       1 << OMAP3_ISP_IOMEM_CSI2C_REGS2,
+		.offset = {
+			/* first MMIO area */
+			0x0000, /* base, len 0x0070 */
+			0x0400, /* ccp2, len 0x01f0 */
+			0x0600, /* ccdc, len 0x00a8 */
+			0x0a00, /* hist, len 0x0048 */
+			0x0c00, /* h3a, len 0x0060 */
+			0x0e00, /* preview, len 0x00a0 */
+			0x1000, /* resizer, len 0x00ac */
+			0x1200, /* sbl, len 0x00fc */
+			/* second MMIO area */
+			0x0000, /* csi2a, len 0x0170 (1st area) */
+			0x0170, /* csiphy2, len 0x000c */
+			0x01c0, /* csi2a, len 0x0040 (2nd area) */
+			0x0400, /* csi2c, len 0x0170 (1st area) */
+			0x0570, /* csiphy1, len 0x000c */
+			0x05c0, /* csi2c, len 0x0040 (2nd area) */
+		},
 		.syscon_offset = 0x2f0,
 		.phy_type = ISP_PHY_TYPE_3630,
 	},
@@ -2235,27 +2243,6 @@ static int isp_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static int isp_map_mem_resource(struct platform_device *pdev,
-				struct isp_device *isp,
-				enum isp_mem_resources res)
-{
-	struct resource *mem;
-
-	/* request the mem region for the camera registers */
-
-	mem = platform_get_resource(pdev, IORESOURCE_MEM, res);
-
-	/* map the region */
-	isp->mmio_base[res] = devm_ioremap_resource(isp->dev, mem);
-	if (IS_ERR(isp->mmio_base[res]))
-		return PTR_ERR(isp->mmio_base[res]);
-
-	if (res == OMAP3_ISP_IOMEM_HIST)
-		isp->mmio_hist_base_phys = mem->start;
-
-	return 0;
-}
-
 /*
  * isp_probe - Probe ISP platform device
  * @pdev: Pointer to ISP platform device
@@ -2271,6 +2258,7 @@ static int isp_probe(struct platform_device *pdev)
 {
 	struct isp_platform_data *pdata = pdev->dev.platform_data;
 	struct isp_device *isp;
+	struct resource *mem;
 	int ret;
 	int i, m;
 
@@ -2303,10 +2291,21 @@ static int isp_probe(struct platform_device *pdev)
 	 *
 	 * The ISP clock tree is revision-dependent. We thus need to enable ICLK
 	 * manually to read the revision before calling __omap3isp_get().
+	 *
+	 * Start by mapping the ISP MMIO area, which is in two pieces.
+	 * The ISP IOMMU is in between. Map both now, and fill in the
+	 * ISP revision specific portions a little later in the
+	 * function.
 	 */
-	ret = isp_map_mem_resource(pdev, isp, OMAP3_ISP_IOMEM_MAIN);
-	if (ret < 0)
-		goto error;
+	for (i = 0; i < 2; i++) {
+		unsigned int map_idx = i ? OMAP3_ISP_IOMEM_CSI2A_REGS1 : 0;
+
+		mem = platform_get_resource(pdev, IORESOURCE_MEM, i);
+		isp->mmio_base[map_idx] =
+			devm_ioremap_resource(isp->dev, mem);
+		if (IS_ERR(isp->mmio_base[map_idx]))
+			return PTR_ERR(isp->mmio_base[map_idx]);
+	}
 
 	ret = isp_get_clocks(isp);
 	if (ret < 0)
@@ -2347,13 +2346,17 @@ static int isp_probe(struct platform_device *pdev)
 		goto error_isp;
 	}
 
-	for (i = 1; i < OMAP3_ISP_IOMEM_LAST; i++) {
-		if (isp_res_maps[m].map & 1 << i) {
-			ret = isp_map_mem_resource(pdev, isp, i);
-			if (ret)
-				goto error_isp;
-		}
-	}
+	for (i = 1; i < OMAP3_ISP_IOMEM_CSI2A_REGS1; i++)
+		isp->mmio_base[i] =
+			isp->mmio_base[0] + isp_res_maps[m].offset[i];
+
+	for (i = OMAP3_ISP_IOMEM_CSIPHY2; i < OMAP3_ISP_IOMEM_LAST; i++)
+		isp->mmio_base[i] =
+			isp->mmio_base[OMAP3_ISP_IOMEM_CSI2A_REGS1]
+			+ isp_res_maps[m].offset[i];
+
+	isp->mmio_hist_base_phys =
+		mem->start + isp_res_maps[m].offset[OMAP3_ISP_IOMEM_HIST];
 
 	isp->syscon = syscon_regmap_lookup_by_pdevname("syscon.0");
 	if (IS_ERR(isp->syscon)) {
diff --git a/drivers/media/platform/omap3isp/isp.h b/drivers/media/platform/omap3isp/isp.h
index 03d2129..dcb7d20 100644
--- a/drivers/media/platform/omap3isp/isp.h
+++ b/drivers/media/platform/omap3isp/isp.h
@@ -99,7 +99,7 @@ struct regmap;
 /*
  * struct isp_res_mapping - Map ISP io resources to ISP revision.
  * @isp_rev: ISP_REVISION_x_x
- * @map: bitmap for enum isp_mem_resources
+ * @offset: register offsets of various ISP sub-blocks
  * @syscon_offset: offset of the syscon register for 343x / 3630
  *	    (CONTROL_CSIRXFE / CONTROL_CAMERA_PHY_CTRL, respectively)
  *	    from the syscon base address
@@ -107,7 +107,7 @@ struct regmap;
  */
 struct isp_res_mapping {
 	u32 isp_rev;
-	u32 map;
+	u32 offset[OMAP3_ISP_IOMEM_LAST];
 	u32 syscon_offset;
 	u32 phy_type;
 };
-- 
1.7.10.4

[PATCH 12/15] dt: bindings: Add lane-polarity property to endpoint nodes

[Sakari Ailus]

Add lane-polarity property to endpoint nodes. This essentially tells that
the order of the differential signal wires is inverted.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
---
 Documentation/devicetree/bindings/media/video-interfaces.txt |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
index 571b4c6..27cfa4e 100644
--- a/Documentation/devicetree/bindings/media/video-interfaces.txt
+++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
@@ -106,6 +106,12 @@ Optional endpoint properties
 - link-frequencies: Allowed data bus frequencies. For MIPI CSI-2, for
   instance, this is the actual frequency of the bus, not bits per clock per
   lane value. An array of 64-bit unsigned integers.
+- lane-polarity: an array of polarities of the lanes starting from the clock
+  lane and followed by the data lanes in the same order as in data-lanes.
+  Valid values are 0 (normal) and 1 (inverted). The length of the array
+  should be the combined length of data-lanes and clock-lanes properties.
+  If the lane-polarity property is omitted, the value must be interpreted as 0
+  (normal). This property is valid for serial busses only.
 
 
 Example
-- 
1.7.10.4

Re: [PATCH 12/15] dt: bindings: Add lane-polarity property to endpoint nodes

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Monday 16 March 2015 02:26:07 Sakari Ailus wrote:
> Add lane-polarity property to endpoint nodes. This essentially tells that
> the order of the differential signal wires is inverted.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  Documentation/devicetree/bindings/media/video-interfaces.txt |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt
> b/Documentation/devicetree/bindings/media/video-interfaces.txt index
> 571b4c6..27cfa4e 100644
> --- a/Documentation/devicetree/bindings/media/video-interfaces.txt
> +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
> @@ -106,6 +106,12 @@ Optional endpoint properties
>  - link-frequencies: Allowed data bus frequencies. For MIPI CSI-2, for
>    instance, this is the actual frequency of the bus, not bits per clock per
> lane value. An array of 64-bit unsigned integers.
> +- lane-polarity: an array of polarities of the lanes starting from the
> clock +  lane and followed by the data lanes in the same order as in
> data-lanes. +  Valid values are 0 (normal) and 1 (inverted). The length of
> the array +  should be the combined length of data-lanes and clock-lanes
> properties. +  If the lane-polarity property is omitted, the value must be
> interpreted as 0 +  (normal). This property is valid for serial busses
> only.
> 
> 
>  Example

-- 
Regards,

Laurent Pinchart

[PATCH v1.1 12/15] dt: bindings: Add lane-polarity property to endpoint nodes

[Sakari Ailus]

Add lane-polarity property to endpoint nodes. This essentially tells that
the order of the differential signal wires is inverted.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
since v1:

- Rename lane-polarity property as lane-polarities.

 Documentation/devicetree/bindings/media/video-interfaces.txt |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
index 571b4c6..9cd2a36 100644
--- a/Documentation/devicetree/bindings/media/video-interfaces.txt
+++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
@@ -106,6 +106,12 @@ Optional endpoint properties
 - link-frequencies: Allowed data bus frequencies. For MIPI CSI-2, for
   instance, this is the actual frequency of the bus, not bits per clock per
   lane value. An array of 64-bit unsigned integers.
+- lane-polarities: an array of polarities of the lanes starting from the clock
+  lane and followed by the data lanes in the same order as in data-lanes.
+  Valid values are 0 (normal) and 1 (inverted). The length of the array
+  should be the combined length of data-lanes and clock-lanes properties.
+  If the lane-polarities property is omitted, the value must be interpreted
+  as 0 (normal). This property is valid for serial busses only.
 
 
 Example
-- 
1.7.10.4

[PATCH 13/15] v4l: of: Read lane-polarity endpoint property

[Sakari Ailus]

Add lane_polarity field to struct v4l2_of_bus_mipi_csi2 and write the
contents of the lane polarity property to it. The field tells the polarity
of the physical lanes starting from the first one. Any unused lanes are
ignored, i.e. only the polarity of the used lanes is specified.

Also rework reading the "data-lanes" property a little.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
---
 drivers/media/v4l2-core/v4l2-of.c |   41 +++++++++++++++++++++++++++++--------
 include/media/v4l2-of.h           |    3 +++
 2 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-of.c b/drivers/media/v4l2-core/v4l2-of.c
index b4ed9a9..e44cc15 100644
--- a/drivers/media/v4l2-core/v4l2-of.c
+++ b/drivers/media/v4l2-core/v4l2-of.c
@@ -19,11 +19,10 @@
 
 #include <media/v4l2-of.h>
 
-static void v4l2_of_parse_csi_bus(const struct device_node *node,
-				  struct v4l2_of_endpoint *endpoint)
+static int v4l2_of_parse_csi_bus(const struct device_node *node,
+				 struct v4l2_of_endpoint *endpoint)
 {
 	struct v4l2_of_bus_mipi_csi2 *bus = &endpoint->bus.mipi_csi2;
-	u32 data_lanes[ARRAY_SIZE(bus->data_lanes)];
 	struct property *prop;
 	bool have_clk_lane = false;
 	unsigned int flags = 0;
@@ -32,16 +31,34 @@ static void v4l2_of_parse_csi_bus(const struct device_node *node,
 	prop = of_find_property(node, "data-lanes", NULL);
 	if (prop) {
 		const __be32 *lane = NULL;
-		int i;
+		unsigned int i;
 
-		for (i = 0; i < ARRAY_SIZE(data_lanes); i++) {
-			lane = of_prop_next_u32(prop, lane, &data_lanes[i]);
+		for (i = 0; i < ARRAY_SIZE(bus->data_lanes); i++) {
+			lane = of_prop_next_u32(prop, lane, &v);
 			if (!lane)
 				break;
+			bus->data_lanes[i] = v;
 		}
 		bus->num_data_lanes = i;
-		while (i--)
-			bus->data_lanes[i] = data_lanes[i];
+	}
+
+	prop = of_find_property(node, "lane-polarity", NULL);
+	if (prop) {
+		const __be32 *polarity = NULL;
+		unsigned int i;
+
+		for (i = 0; i < ARRAY_SIZE(bus->lane_polarity); i++) {
+			polarity = of_prop_next_u32(prop, polarity, &v);
+			if (!polarity)
+				break;
+			bus->lane_polarity[i] = v;
+		}
+
+		if (i < 1 + bus->num_data_lanes /* clock + data */) {
+			pr_warn("bad size of lane-polarity array in node %s, was %u, should be %u\n",
+				node->full_name, i, 1 + bus->num_data_lanes);
+			return -EINVAL;
+		}
 	}
 
 	if (!of_property_read_u32(node, "clock-lanes", &v)) {
@@ -56,6 +73,8 @@ static void v4l2_of_parse_csi_bus(const struct device_node *node,
 
 	bus->flags = flags;
 	endpoint->bus_type = V4L2_MBUS_CSI2;
+
+	return 0;
 }
 
 static void v4l2_of_parse_parallel_bus(const struct device_node *node,
@@ -127,11 +146,15 @@ static void v4l2_of_parse_parallel_bus(const struct device_node *node,
 int v4l2_of_parse_endpoint(const struct device_node *node,
 			   struct v4l2_of_endpoint *endpoint)
 {
+	int rval;
+
 	of_graph_parse_endpoint(node, &endpoint->base);
 	endpoint->bus_type = 0;
 	memset(&endpoint->bus, 0, sizeof(endpoint->bus));
 
-	v4l2_of_parse_csi_bus(node, endpoint);
+	rval = v4l2_of_parse_csi_bus(node, endpoint);
+	if (rval)
+		return rval;
 	/*
 	 * Parse the parallel video bus properties only if none
 	 * of the MIPI CSI-2 specific properties were found.
diff --git a/include/media/v4l2-of.h b/include/media/v4l2-of.h
index 70fa7b7..a70eb52 100644
--- a/include/media/v4l2-of.h
+++ b/include/media/v4l2-of.h
@@ -29,12 +29,15 @@ struct device_node;
  * @data_lanes: an array of physical data lane indexes
  * @clock_lane: physical lane index of the clock lane
  * @num_data_lanes: number of data lanes
+ * @lane_polarity: polarity of the lanes. The order is the same of
+ *		   the physical lanes.
  */
 struct v4l2_of_bus_mipi_csi2 {
 	unsigned int flags;
 	unsigned char data_lanes[4];
 	unsigned char clock_lane;
 	unsigned short num_data_lanes;
+	bool lane_polarity[5];
 };
 
 /**
-- 
1.7.10.4

Re: [PATCH 13/15] v4l: of: Read lane-polarity endpoint property

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Monday 16 March 2015 02:26:08 Sakari Ailus wrote:
> Add lane_polarity field to struct v4l2_of_bus_mipi_csi2 and write the
> contents of the lane polarity property to it. The field tells the polarity
> of the physical lanes starting from the first one. Any unused lanes are
> ignored, i.e. only the polarity of the used lanes is specified.
> 
> Also rework reading the "data-lanes" property a little.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> ---
>  drivers/media/v4l2-core/v4l2-of.c |   41 ++++++++++++++++++++++++++--------
>  include/media/v4l2-of.h           |    3 +++
>  2 files changed, 35 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-of.c
> b/drivers/media/v4l2-core/v4l2-of.c index b4ed9a9..e44cc15 100644
> --- a/drivers/media/v4l2-core/v4l2-of.c
> +++ b/drivers/media/v4l2-core/v4l2-of.c
> @@ -19,11 +19,10 @@
> 
>  #include <media/v4l2-of.h>
> 
> -static void v4l2_of_parse_csi_bus(const struct device_node *node,
> -				  struct v4l2_of_endpoint *endpoint)
> +static int v4l2_of_parse_csi_bus(const struct device_node *node,
> +				 struct v4l2_of_endpoint *endpoint)
>  {
>  	struct v4l2_of_bus_mipi_csi2 *bus = &endpoint->bus.mipi_csi2;
> -	u32 data_lanes[ARRAY_SIZE(bus->data_lanes)];
>  	struct property *prop;
>  	bool have_clk_lane = false;
>  	unsigned int flags = 0;
> @@ -32,16 +31,34 @@ static void v4l2_of_parse_csi_bus(const struct
> device_node *node, prop = of_find_property(node, "data-lanes", NULL);
>  	if (prop) {
>  		const __be32 *lane = NULL;
> -		int i;
> +		unsigned int i;
> 
> -		for (i = 0; i < ARRAY_SIZE(data_lanes); i++) {
> -			lane = of_prop_next_u32(prop, lane, &data_lanes[i]);
> +		for (i = 0; i < ARRAY_SIZE(bus->data_lanes); i++) {
> +			lane = of_prop_next_u32(prop, lane, &v);
>  			if (!lane)
>  				break;
> +			bus->data_lanes[i] = v;
>  		}
>  		bus->num_data_lanes = i;
> -		while (i--)
> -			bus->data_lanes[i] = data_lanes[i];
> +	}
> +
> +	prop = of_find_property(node, "lane-polarity", NULL);
> +	if (prop) {
> +		const __be32 *polarity = NULL;
> +		unsigned int i;
> +
> +		for (i = 0; i < ARRAY_SIZE(bus->lane_polarity); i++) {
> +			polarity = of_prop_next_u32(prop, polarity, &v);
> +			if (!polarity)
> +				break;
> +			bus->lane_polarity[i] = v;
> +		}
> +
> +		if (i < 1 + bus->num_data_lanes /* clock + data */) {
> +			pr_warn("bad size of lane-polarity array in node %s, was %u, 
should be
> %u\n",

How about

		pr_warn("%s: too few lane-polarity entries (need %u, got %u)\n",
			node->full_name, 1 + bus->num_data_lanes, i);

> +				node->full_name, i, 1 + bus->num_data_lanes);
> +			return -EINVAL;
> +		}
>  	}
> 
>  	if (!of_property_read_u32(node, "clock-lanes", &v)) {
> @@ -56,6 +73,8 @@ static void v4l2_of_parse_csi_bus(const struct device_node
> *node,
> 
>  	bus->flags = flags;
>  	endpoint->bus_type = V4L2_MBUS_CSI2;
> +
> +	return 0;
>  }
> 
>  static void v4l2_of_parse_parallel_bus(const struct device_node *node,
> @@ -127,11 +146,15 @@ static void v4l2_of_parse_parallel_bus(const struct
> device_node *node, int v4l2_of_parse_endpoint(const struct device_node
> *node,
>  			   struct v4l2_of_endpoint *endpoint)
>  {
> +	int rval;
> +
>  	of_graph_parse_endpoint(node, &endpoint->base);
>  	endpoint->bus_type = 0;
>  	memset(&endpoint->bus, 0, sizeof(endpoint->bus));
> 
> -	v4l2_of_parse_csi_bus(node, endpoint);
> +	rval = v4l2_of_parse_csi_bus(node, endpoint);
> +	if (rval)
> +		return rval;
>  	/*
>  	 * Parse the parallel video bus properties only if none
>  	 * of the MIPI CSI-2 specific properties were found.
> diff --git a/include/media/v4l2-of.h b/include/media/v4l2-of.h
> index 70fa7b7..a70eb52 100644
> --- a/include/media/v4l2-of.h
> +++ b/include/media/v4l2-of.h
> @@ -29,12 +29,15 @@ struct device_node;
>   * @data_lanes: an array of physical data lane indexes
>   * @clock_lane: physical lane index of the clock lane
>   * @num_data_lanes: number of data lanes
> + * @lane_polarity: polarity of the lanes. The order is the same of
> + *		   the physical lanes.
>   */
>  struct v4l2_of_bus_mipi_csi2 {
>  	unsigned int flags;
>  	unsigned char data_lanes[4];
>  	unsigned char clock_lane;
>  	unsigned short num_data_lanes;
> +	bool lane_polarity[5];

A bit of bike-shedding here, should this be lane_polarities ? And, thinking 
about it, should the DT property be renamed to "lane-polarities" as well ? 
This would match "data-lanes".

>  };
> 
>  /**

-- 
Regards,

Laurent Pinchart

Re: [PATCH 13/15] v4l: of: Read lane-polarity endpoint property

[Sakari Ailus]

Hi Laurent,

On Mon, Mar 16, 2015 at 11:05:38AM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Monday 16 March 2015 02:26:08 Sakari Ailus wrote:
> > Add lane_polarity field to struct v4l2_of_bus_mipi_csi2 and write the
> > contents of the lane polarity property to it. The field tells the polarity
> > of the physical lanes starting from the first one. Any unused lanes are
> > ignored, i.e. only the polarity of the used lanes is specified.
> > 
> > Also rework reading the "data-lanes" property a little.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> > ---
> >  drivers/media/v4l2-core/v4l2-of.c |   41 ++++++++++++++++++++++++++--------
> >  include/media/v4l2-of.h           |    3 +++
> >  2 files changed, 35 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-of.c
> > b/drivers/media/v4l2-core/v4l2-of.c index b4ed9a9..e44cc15 100644
> > --- a/drivers/media/v4l2-core/v4l2-of.c
> > +++ b/drivers/media/v4l2-core/v4l2-of.c
> > @@ -19,11 +19,10 @@
> > 
> >  #include <media/v4l2-of.h>
> > 
> > -static void v4l2_of_parse_csi_bus(const struct device_node *node,
> > -				  struct v4l2_of_endpoint *endpoint)
> > +static int v4l2_of_parse_csi_bus(const struct device_node *node,
> > +				 struct v4l2_of_endpoint *endpoint)
> >  {
> >  	struct v4l2_of_bus_mipi_csi2 *bus = &endpoint->bus.mipi_csi2;
> > -	u32 data_lanes[ARRAY_SIZE(bus->data_lanes)];
> >  	struct property *prop;
> >  	bool have_clk_lane = false;
> >  	unsigned int flags = 0;
> > @@ -32,16 +31,34 @@ static void v4l2_of_parse_csi_bus(const struct
> > device_node *node, prop = of_find_property(node, "data-lanes", NULL);
> >  	if (prop) {
> >  		const __be32 *lane = NULL;
> > -		int i;
> > +		unsigned int i;
> > 
> > -		for (i = 0; i < ARRAY_SIZE(data_lanes); i++) {
> > -			lane = of_prop_next_u32(prop, lane, &data_lanes[i]);
> > +		for (i = 0; i < ARRAY_SIZE(bus->data_lanes); i++) {
> > +			lane = of_prop_next_u32(prop, lane, &v);
> >  			if (!lane)
> >  				break;
> > +			bus->data_lanes[i] = v;
> >  		}
> >  		bus->num_data_lanes = i;
> > -		while (i--)
> > -			bus->data_lanes[i] = data_lanes[i];
> > +	}
> > +
> > +	prop = of_find_property(node, "lane-polarity", NULL);
> > +	if (prop) {
> > +		const __be32 *polarity = NULL;
> > +		unsigned int i;
> > +
> > +		for (i = 0; i < ARRAY_SIZE(bus->lane_polarity); i++) {
> > +			polarity = of_prop_next_u32(prop, polarity, &v);
> > +			if (!polarity)
> > +				break;
> > +			bus->lane_polarity[i] = v;
> > +		}
> > +
> > +		if (i < 1 + bus->num_data_lanes /* clock + data */) {
> > +			pr_warn("bad size of lane-polarity array in node %s, was %u, 
> should be
> > %u\n",
> 
> How about
> 
> 		pr_warn("%s: too few lane-polarity entries (need %u, got %u)\n",
> 			node->full_name, 1 + bus->num_data_lanes, i);

Fixed.

> > +				node->full_name, i, 1 + bus->num_data_lanes);
> > +			return -EINVAL;
> > +		}
> >  	}
> > 
> >  	if (!of_property_read_u32(node, "clock-lanes", &v)) {
> > @@ -56,6 +73,8 @@ static void v4l2_of_parse_csi_bus(const struct device_node
> > *node,
> > 
> >  	bus->flags = flags;
> >  	endpoint->bus_type = V4L2_MBUS_CSI2;
> > +
> > +	return 0;
> >  }
> > 
> >  static void v4l2_of_parse_parallel_bus(const struct device_node *node,
> > @@ -127,11 +146,15 @@ static void v4l2_of_parse_parallel_bus(const struct
> > device_node *node, int v4l2_of_parse_endpoint(const struct device_node
> > *node,
> >  			   struct v4l2_of_endpoint *endpoint)
> >  {
> > +	int rval;
> > +
> >  	of_graph_parse_endpoint(node, &endpoint->base);
> >  	endpoint->bus_type = 0;
> >  	memset(&endpoint->bus, 0, sizeof(endpoint->bus));
> > 
> > -	v4l2_of_parse_csi_bus(node, endpoint);
> > +	rval = v4l2_of_parse_csi_bus(node, endpoint);
> > +	if (rval)
> > +		return rval;
> >  	/*
> >  	 * Parse the parallel video bus properties only if none
> >  	 * of the MIPI CSI-2 specific properties were found.
> > diff --git a/include/media/v4l2-of.h b/include/media/v4l2-of.h
> > index 70fa7b7..a70eb52 100644
> > --- a/include/media/v4l2-of.h
> > +++ b/include/media/v4l2-of.h
> > @@ -29,12 +29,15 @@ struct device_node;
> >   * @data_lanes: an array of physical data lane indexes
> >   * @clock_lane: physical lane index of the clock lane
> >   * @num_data_lanes: number of data lanes
> > + * @lane_polarity: polarity of the lanes. The order is the same of
> > + *		   the physical lanes.
> >   */
> >  struct v4l2_of_bus_mipi_csi2 {
> >  	unsigned int flags;
> >  	unsigned char data_lanes[4];
> >  	unsigned char clock_lane;
> >  	unsigned short num_data_lanes;
> > +	bool lane_polarity[5];
> 
> A bit of bike-shedding here, should this be lane_polarities ? And, thinking 
> about it, should the DT property be renamed to "lane-polarities" as well ? 
> This would match "data-lanes".

Good idea. I'll take that into account before reposting the sets.

-- 
Cheers,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

[PATCH v1.1 13/15] v4l: of: Read lane-polarities endpoint property

[Sakari Ailus]

Add lane_polarities field to struct v4l2_of_bus_mipi_csi2 and write the
contents of the lane-polarities property to it. The field tells the polarity
of the physical lanes starting from the first one. Any unused lanes are
ignored, i.e. only the polarity of the used lanes is specified.

Also rework reading the "data-lanes" property a little.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
---
since v1:

- Rename lane-polarity property as lane-polarities.

 drivers/media/v4l2-core/v4l2-of.c |   41 +++++++++++++++++++++++++++++--------
 include/media/v4l2-of.h           |    3 +++
 2 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-of.c b/drivers/media/v4l2-core/v4l2-of.c
index b4ed9a9..58e401f 100644
--- a/drivers/media/v4l2-core/v4l2-of.c
+++ b/drivers/media/v4l2-core/v4l2-of.c
@@ -19,11 +19,10 @@
 
 #include <media/v4l2-of.h>
 
-static void v4l2_of_parse_csi_bus(const struct device_node *node,
-				  struct v4l2_of_endpoint *endpoint)
+static int v4l2_of_parse_csi_bus(const struct device_node *node,
+				 struct v4l2_of_endpoint *endpoint)
 {
 	struct v4l2_of_bus_mipi_csi2 *bus = &endpoint->bus.mipi_csi2;
-	u32 data_lanes[ARRAY_SIZE(bus->data_lanes)];
 	struct property *prop;
 	bool have_clk_lane = false;
 	unsigned int flags = 0;
@@ -32,16 +31,34 @@ static void v4l2_of_parse_csi_bus(const struct device_node *node,
 	prop = of_find_property(node, "data-lanes", NULL);
 	if (prop) {
 		const __be32 *lane = NULL;
-		int i;
+		unsigned int i;
 
-		for (i = 0; i < ARRAY_SIZE(data_lanes); i++) {
-			lane = of_prop_next_u32(prop, lane, &data_lanes[i]);
+		for (i = 0; i < ARRAY_SIZE(bus->data_lanes); i++) {
+			lane = of_prop_next_u32(prop, lane, &v);
 			if (!lane)
 				break;
+			bus->data_lanes[i] = v;
 		}
 		bus->num_data_lanes = i;
-		while (i--)
-			bus->data_lanes[i] = data_lanes[i];
+	}
+
+	prop = of_find_property(node, "lane-polarities", NULL);
+	if (prop) {
+		const __be32 *polarity = NULL;
+		unsigned int i;
+
+		for (i = 0; i < ARRAY_SIZE(bus->lane_polarities); i++) {
+			polarity = of_prop_next_u32(prop, polarity, &v);
+			if (!polarity)
+				break;
+			bus->lane_polarities[i] = v;
+		}
+
+		if (i < 1 + bus->num_data_lanes /* clock + data */) {
+			pr_warn("%s: too few lane-polarities entries (need %u, got %u)\n",
+				node->full_name, 1 + bus->num_data_lanes, i);
+			return -EINVAL;
+		}
 	}
 
 	if (!of_property_read_u32(node, "clock-lanes", &v)) {
@@ -56,6 +73,8 @@ static void v4l2_of_parse_csi_bus(const struct device_node *node,
 
 	bus->flags = flags;
 	endpoint->bus_type = V4L2_MBUS_CSI2;
+
+	return 0;
 }
 
 static void v4l2_of_parse_parallel_bus(const struct device_node *node,
@@ -127,11 +146,15 @@ static void v4l2_of_parse_parallel_bus(const struct device_node *node,
 int v4l2_of_parse_endpoint(const struct device_node *node,
 			   struct v4l2_of_endpoint *endpoint)
 {
+	int rval;
+
 	of_graph_parse_endpoint(node, &endpoint->base);
 	endpoint->bus_type = 0;
 	memset(&endpoint->bus, 0, sizeof(endpoint->bus));
 
-	v4l2_of_parse_csi_bus(node, endpoint);
+	rval = v4l2_of_parse_csi_bus(node, endpoint);
+	if (rval)
+		return rval;
 	/*
 	 * Parse the parallel video bus properties only if none
 	 * of the MIPI CSI-2 specific properties were found.
diff --git a/include/media/v4l2-of.h b/include/media/v4l2-of.h
index 70fa7b7..2de42c5 100644
--- a/include/media/v4l2-of.h
+++ b/include/media/v4l2-of.h
@@ -29,12 +29,15 @@ struct device_node;
  * @data_lanes: an array of physical data lane indexes
  * @clock_lane: physical lane index of the clock lane
  * @num_data_lanes: number of data lanes
+ * @lane_polarities: polarity of the lanes. The order is the same of
+ *		   the physical lanes.
  */
 struct v4l2_of_bus_mipi_csi2 {
 	unsigned int flags;
 	unsigned char data_lanes[4];
 	unsigned char clock_lane;
 	unsigned short num_data_lanes;
+	bool lane_polarities[5];
 };
 
 /**
-- 
1.7.10.4

Re: [PATCH v1.1 13/15] v4l: of: Read lane-polarities endpoint property

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.
On Saturday 21 March 2015 00:08:15 Sakari Ailus wrote:
> Add lane_polarities field to struct v4l2_of_bus_mipi_csi2 and write the
> contents of the lane-polarities property to it. The field tells the polarity
> of the physical lanes starting from the first one. Any unused lanes are
> ignored, i.e. only the polarity of the used lanes is specified.
> 
> Also rework reading the "data-lanes" property a little.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
> since v1:
> 
> - Rename lane-polarity property as lane-polarities.
> 
>  drivers/media/v4l2-core/v4l2-of.c |   41 ++++++++++++++++++++++++++--------
>  include/media/v4l2-of.h           |    3 +++
>  2 files changed, 35 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-of.c
> b/drivers/media/v4l2-core/v4l2-of.c index b4ed9a9..58e401f 100644
> --- a/drivers/media/v4l2-core/v4l2-of.c
> +++ b/drivers/media/v4l2-core/v4l2-of.c
> @@ -19,11 +19,10 @@
> 
>  #include <media/v4l2-of.h>
> 
> -static void v4l2_of_parse_csi_bus(const struct device_node *node,
> -				  struct v4l2_of_endpoint *endpoint)
> +static int v4l2_of_parse_csi_bus(const struct device_node *node,
> +				 struct v4l2_of_endpoint *endpoint)
>  {
>  	struct v4l2_of_bus_mipi_csi2 *bus = &endpoint->bus.mipi_csi2;
> -	u32 data_lanes[ARRAY_SIZE(bus->data_lanes)];
>  	struct property *prop;
>  	bool have_clk_lane = false;
>  	unsigned int flags = 0;
> @@ -32,16 +31,34 @@ static void v4l2_of_parse_csi_bus(const struct
> device_node *node, prop = of_find_property(node, "data-lanes", NULL);
>  	if (prop) {
>  		const __be32 *lane = NULL;
> -		int i;
> +		unsigned int i;
> 
> -		for (i = 0; i < ARRAY_SIZE(data_lanes); i++) {
> -			lane = of_prop_next_u32(prop, lane, &data_lanes[i]);
> +		for (i = 0; i < ARRAY_SIZE(bus->data_lanes); i++) {
> +			lane = of_prop_next_u32(prop, lane, &v);
>  			if (!lane)
>  				break;
> +			bus->data_lanes[i] = v;
>  		}
>  		bus->num_data_lanes = i;
> -		while (i--)
> -			bus->data_lanes[i] = data_lanes[i];
> +	}
> +
> +	prop = of_find_property(node, "lane-polarities", NULL);
> +	if (prop) {
> +		const __be32 *polarity = NULL;
> +		unsigned int i;
> +
> +		for (i = 0; i < ARRAY_SIZE(bus->lane_polarities); i++) {
> +			polarity = of_prop_next_u32(prop, polarity, &v);
> +			if (!polarity)
> +				break;
> +			bus->lane_polarities[i] = v;
> +		}
> +
> +		if (i < 1 + bus->num_data_lanes /* clock + data */) {
> +			pr_warn("%s: too few lane-polarities entries (need %u, got 
%u)\n",
> +				node->full_name, 1 + bus->num_data_lanes, i);
> +			return -EINVAL;
> +		}
>  	}
> 
>  	if (!of_property_read_u32(node, "clock-lanes", &v)) {
> @@ -56,6 +73,8 @@ static void v4l2_of_parse_csi_bus(const struct device_node
> *node,
> 
>  	bus->flags = flags;
>  	endpoint->bus_type = V4L2_MBUS_CSI2;
> +
> +	return 0;
>  }
> 
>  static void v4l2_of_parse_parallel_bus(const struct device_node *node,
> @@ -127,11 +146,15 @@ static void v4l2_of_parse_parallel_bus(const struct
> device_node *node, int v4l2_of_parse_endpoint(const struct device_node
> *node,
>  			   struct v4l2_of_endpoint *endpoint)
>  {
> +	int rval;
> +
>  	of_graph_parse_endpoint(node, &endpoint->base);
>  	endpoint->bus_type = 0;
>  	memset(&endpoint->bus, 0, sizeof(endpoint->bus));
> 
> -	v4l2_of_parse_csi_bus(node, endpoint);
> +	rval = v4l2_of_parse_csi_bus(node, endpoint);
> +	if (rval)
> +		return rval;
>  	/*
>  	 * Parse the parallel video bus properties only if none
>  	 * of the MIPI CSI-2 specific properties were found.
> diff --git a/include/media/v4l2-of.h b/include/media/v4l2-of.h
> index 70fa7b7..2de42c5 100644
> --- a/include/media/v4l2-of.h
> +++ b/include/media/v4l2-of.h
> @@ -29,12 +29,15 @@ struct device_node;
>   * @data_lanes: an array of physical data lane indexes
>   * @clock_lane: physical lane index of the clock lane
>   * @num_data_lanes: number of data lanes
> + * @lane_polarities: polarity of the lanes. The order is the same of
> + *		   the physical lanes.
>   */
>  struct v4l2_of_bus_mipi_csi2 {
>  	unsigned int flags;
>  	unsigned char data_lanes[4];
>  	unsigned char clock_lane;
>  	unsigned short num_data_lanes;
> +	bool lane_polarities[5];
>  };
> 
>  /**

-- 
Regards,

Laurent Pinchart

[PATCH 14/15] omap3isp: Add support for the Device Tree

[Sakari Ailus]

Add the ISP device to omap3 DT include file and add support to the driver to
use it.

Also obtain information on the external entities and the ISP configuration
related to them through the Device Tree in addition to the platform data.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
---
 drivers/media/platform/omap3isp/isp.c       |  209 +++++++++++++++++++++++++--
 drivers/media/platform/omap3isp/isp.h       |   11 ++
 drivers/media/platform/omap3isp/ispcsiphy.c |    7 +
 3 files changed, 215 insertions(+), 12 deletions(-)

diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 992e74c..0d6012a 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -64,6 +64,7 @@
 
 #include <media/v4l2-common.h>
 #include <media/v4l2-device.h>
+#include <media/v4l2-of.h>
 
 #include "isp.h"
 #include "ispreg.h"
@@ -1991,6 +1992,14 @@ static int isp_register_entities(struct isp_device *isp)
 	if (ret < 0)
 		goto done;
 
+	/*
+	 * Device Tree --- the external of the sub-devices will be
+	 * registered later. Same goes for the sub-device node
+	 * registration.
+	 */
+	if (isp->dev->of_node)
+		return 0;
+
 	/* Register external entities */
 	for (isp_subdev = pdata ? pdata->subdevs : NULL;
 	     isp_subdev && isp_subdev->board_info; isp_subdev++) {
@@ -2016,8 +2025,10 @@ static int isp_register_entities(struct isp_device *isp)
 	ret = v4l2_device_register_subdev_nodes(&isp->v4l2_dev);
 
 done:
-	if (ret < 0)
+	if (ret < 0) {
 		isp_unregister_entities(isp);
+		v4l2_async_notifier_unregister(&isp->notifier);
+	}
 
 	return ret;
 }
@@ -2232,6 +2243,7 @@ static int isp_remove(struct platform_device *pdev)
 {
 	struct isp_device *isp = platform_get_drvdata(pdev);
 
+	v4l2_async_notifier_unregister(&isp->notifier);
 	isp_unregister_entities(isp);
 	isp_cleanup_modules(isp);
 	isp_xclk_cleanup(isp);
@@ -2243,6 +2255,151 @@ static int isp_remove(struct platform_device *pdev)
 	return 0;
 }
 
+enum isp_of_phy {
+	ISP_OF_PHY_PARALLEL = 0,
+	ISP_OF_PHY_CSIPHY1,
+	ISP_OF_PHY_CSIPHY2,
+};
+
+static int isp_of_parse_node(struct device *dev, struct device_node *node,
+			     struct isp_async_subdev *isd)
+{
+	struct isp_bus_cfg *buscfg = &isd->bus;
+	struct v4l2_of_endpoint vep;
+	unsigned int i;
+
+	v4l2_of_parse_endpoint(node, &vep);
+
+	dev_dbg(dev, "interface %u\n", vep.base.port);
+
+	switch (vep.base.port) {
+	case ISP_OF_PHY_PARALLEL:
+		buscfg->interface = ISP_INTERFACE_PARALLEL;
+		buscfg->bus.parallel.data_lane_shift =
+			vep.bus.parallel.data_shift;
+		buscfg->bus.parallel.clk_pol =
+			!!(vep.bus.parallel.flags
+			   & V4L2_MBUS_PCLK_SAMPLE_FALLING);
+		buscfg->bus.parallel.hs_pol =
+			!!(vep.bus.parallel.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW);
+		buscfg->bus.parallel.vs_pol =
+			!!(vep.bus.parallel.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW);
+		buscfg->bus.parallel.fld_pol =
+			!!(vep.bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
+		buscfg->bus.parallel.data_pol =
+			!!(vep.bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
+		break;
+
+	case ISP_OF_PHY_CSIPHY1:
+	case ISP_OF_PHY_CSIPHY2:
+		/* FIXME: always assume CSI-2 for now. */
+		switch (vep.base.port) {
+		case ISP_OF_PHY_CSIPHY1:
+			buscfg->interface = ISP_INTERFACE_CSI2C_PHY1;
+			break;
+		case ISP_OF_PHY_CSIPHY2:
+			buscfg->interface = ISP_INTERFACE_CSI2A_PHY2;
+			break;
+		}
+		buscfg->bus.csi2.lanecfg.clk.pos = vep.bus.mipi_csi2.clock_lane;
+		buscfg->bus.csi2.lanecfg.clk.pol =
+			vep.bus.mipi_csi2.lane_polarity[0];
+		dev_dbg(dev, "clock lane polarity %u, pos %u\n",
+			buscfg->bus.csi2.lanecfg.clk.pol,
+			buscfg->bus.csi2.lanecfg.clk.pos);
+
+		for (i = 0; i < ISP_CSIPHY2_NUM_DATA_LANES; i++) {
+			buscfg->bus.csi2.lanecfg.data[i].pos =
+				vep.bus.mipi_csi2.data_lanes[i];
+			buscfg->bus.csi2.lanecfg.data[i].pol =
+				vep.bus.mipi_csi2.lane_polarity[i + 1];
+			dev_dbg(dev, "data lane %u polarity %u, pos %u\n", i,
+				buscfg->bus.csi2.lanecfg.data[i].pol,
+				buscfg->bus.csi2.lanecfg.data[i].pos);
+		}
+
+		/*
+		 * FIXME: now we assume the CRC is always there.
+		 * Implement a way to obtain this information from the
+		 * sensor. Frame descriptors, perhaps?
+		 */
+		buscfg->bus.csi2.crc = 1;
+		break;
+
+	default:
+		dev_warn(dev, "invalid interface %d\n\n", vep.base.port);
+		break;
+	}
+
+	return 0;
+}
+
+static int isp_of_parse_nodes(struct device *dev,
+			      struct v4l2_async_notifier *notifier)
+{
+	struct device_node *node = NULL;
+	struct v4l2_async_subdev **asds;
+
+	asds = notifier->subdevs =
+		devm_kcalloc(dev, ISP_MAX_SUBDEVS, sizeof(*notifier->subdevs),
+			     GFP_KERNEL);
+	if (!asds)
+		return -ENOMEM;
+
+	while ((node = of_graph_get_next_endpoint(dev->of_node, node)) &&
+	       notifier->num_subdevs < ISP_MAX_SUBDEVS) {
+		struct isp_async_subdev *isd;
+
+		isd = devm_kzalloc(dev, sizeof(*isd), GFP_KERNEL);
+		if (!isd)
+			return -ENOMEM;
+
+		*asds = &isd->asd;
+
+		if (isp_of_parse_node(dev, node, isd))
+			return -EINVAL;
+
+		isd->asd.match.of.node = of_graph_get_remote_port_parent(node);
+
+		if (!isd->asd.match.of.node) {
+			dev_warn(dev, "bad remote port parent\n");
+			return -EINVAL;
+		}
+		isd->asd.match_type = V4L2_ASYNC_MATCH_OF;
+		notifier->num_subdevs++;
+	}
+
+	return notifier->num_subdevs;
+}
+
+static int isp_subdev_notifier_bound(struct v4l2_async_notifier *async,
+				     struct v4l2_subdev *subdev,
+				     struct v4l2_async_subdev *asd)
+{
+	struct isp_device *isp = container_of(async, struct isp_device,
+					      notifier);
+	struct isp_async_subdev *isd =
+		container_of(asd, struct isp_async_subdev, asd);
+	int rval;
+
+	rval = isp_link_entity(isp, &subdev->entity, isd->bus.interface);
+	if (rval < 0)
+		return rval;
+
+	isd->sd = subdev;
+	isd->sd->host_priv = &isd->bus;
+
+	return rval;
+}
+
+static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
+{
+	struct isp_device *isp = container_of(async, struct isp_device,
+					      notifier);
+
+	return v4l2_device_register_subdev_nodes(&isp->v4l2_dev);
+}
+
 /*
  * isp_probe - Probe ISP platform device
  * @pdev: Pointer to ISP platform device
@@ -2256,7 +2413,6 @@ static int isp_remove(struct platform_device *pdev)
  */
 static int isp_probe(struct platform_device *pdev)
 {
-	struct isp_platform_data *pdata = pdev->dev.platform_data;
 	struct isp_device *isp;
 	struct resource *mem;
 	int ret;
@@ -2268,13 +2424,37 @@ static int isp_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	if (IS_ENABLED(CONFIG_OF) && pdev->dev.of_node) {
+		ret = of_property_read_u32(pdev->dev.of_node, "ti,phy-type",
+					   &isp->phy_type);
+		if (ret)
+			return ret;
+
+		isp->syscon = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
+							      "syscon");
+		if (IS_ERR(isp->syscon))
+			return PTR_ERR(isp->syscon);
+
+		ret = isp_of_parse_nodes(&pdev->dev, &isp->notifier);
+		if (ret < 0)
+			return ret;
+		ret = v4l2_async_notifier_register(&isp->v4l2_dev,
+						   &isp->notifier);
+		if (ret)
+			return ret;
+	} else {
+		isp->pdata = pdev->dev.platform_data;
+		isp->syscon = syscon_regmap_lookup_by_pdevname("syscon.0");
+		if (IS_ERR(isp->syscon))
+			return PTR_ERR(isp->syscon);
+	}
+
 	isp->autoidle = autoidle;
 
 	mutex_init(&isp->isp_mutex);
 	spin_lock_init(&isp->stat_lock);
 
 	isp->dev = &pdev->dev;
-	isp->pdata = pdata;
 	isp->ref_count = 0;
 
 	ret = dma_coerce_mask_and_coherent(isp->dev, DMA_BIT_MASK(32));
@@ -2346,6 +2526,11 @@ static int isp_probe(struct platform_device *pdev)
 		goto error_isp;
 	}
 
+	if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node) {
+		isp->syscon_offset = isp_res_maps[m].syscon_offset;
+		isp->phy_type = isp_res_maps[m].phy_type;
+	}
+
 	for (i = 1; i < OMAP3_ISP_IOMEM_CSI2A_REGS1; i++)
 		isp->mmio_base[i] =
 			isp->mmio_base[0] + isp_res_maps[m].offset[i];
@@ -2358,15 +2543,6 @@ static int isp_probe(struct platform_device *pdev)
 	isp->mmio_hist_base_phys =
 		mem->start + isp_res_maps[m].offset[OMAP3_ISP_IOMEM_HIST];
 
-	isp->syscon = syscon_regmap_lookup_by_pdevname("syscon.0");
-	if (IS_ERR(isp->syscon)) {
-		ret = PTR_ERR(isp->syscon);
-		goto error_isp;
-	}
-
-	isp->syscon_offset = isp_res_maps[m].syscon_offset;
-	isp->phy_type = isp_res_maps[m].phy_type;
-
 	/* IOMMU */
 	ret = isp_attach_iommu(isp);
 	if (ret < 0) {
@@ -2394,6 +2570,9 @@ static int isp_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto error_iommu;
 
+	isp->notifier.bound = isp_subdev_notifier_bound;
+	isp->notifier.complete = isp_subdev_notifier_complete;
+
 	ret = isp_register_entities(isp);
 	if (ret < 0)
 		goto error_modules;
@@ -2429,6 +2608,11 @@ static struct platform_device_id omap3isp_id_table[] = {
 };
 MODULE_DEVICE_TABLE(platform, omap3isp_id_table);
 
+static const struct of_device_id omap3isp_of_table[] = {
+	{ .compatible = "ti,omap3-isp" },
+	{ },
+};
+
 static struct platform_driver omap3isp_driver = {
 	.probe = isp_probe,
 	.remove = isp_remove,
@@ -2436,6 +2620,7 @@ static struct platform_driver omap3isp_driver = {
 	.driver = {
 		.name = "omap3isp",
 		.pm	= &omap3isp_pm_ops,
+		.of_match_table = omap3isp_of_table,
 	},
 };
 
diff --git a/drivers/media/platform/omap3isp/isp.h b/drivers/media/platform/omap3isp/isp.h
index dcb7d20..431224e 100644
--- a/drivers/media/platform/omap3isp/isp.h
+++ b/drivers/media/platform/omap3isp/isp.h
@@ -18,6 +18,7 @@
 #define OMAP3_ISP_CORE_H
 
 #include <media/omap3isp.h>
+#include <media/v4l2-async.h>
 #include <media/v4l2-device.h>
 #include <linux/clk-provider.h>
 #include <linux/device.h>
@@ -178,6 +179,7 @@ struct isp_xclk {
  */
 struct isp_device {
 	struct v4l2_device v4l2_dev;
+	struct v4l2_async_notifier notifier;
 	struct media_device media_dev;
 	struct device *dev;
 	u32 revision;
@@ -224,6 +226,15 @@ struct isp_device {
 
 	unsigned int sbl_resources;
 	unsigned int subclk_resources;
+
+#define ISP_MAX_SUBDEVS		8
+	struct v4l2_subdev *subdevs[ISP_MAX_SUBDEVS];
+};
+
+struct isp_async_subdev {
+	struct v4l2_subdev *sd;
+	struct isp_bus_cfg bus;
+	struct v4l2_async_subdev asd;
 };
 
 #define v4l2_dev_to_isp_device(dev) \
diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c b/drivers/media/platform/omap3isp/ispcsiphy.c
index d91dde1..495447d 100644
--- a/drivers/media/platform/omap3isp/ispcsiphy.c
+++ b/drivers/media/platform/omap3isp/ispcsiphy.c
@@ -173,6 +173,13 @@ static int omap3isp_csiphy_config(struct isp_csiphy *phy)
 	unsigned int i;
 	u32 reg;
 
+	if (!buscfg) {
+		struct isp_async_subdev *isd =
+			container_of(pipe->external->asd,
+				     struct isp_async_subdev, asd);
+		buscfg = &isd->bus;
+	}
+
 	if (buscfg->interface == ISP_INTERFACE_CCP2B_PHY1
 	    || buscfg->interface == ISP_INTERFACE_CCP2B_PHY2)
 		lanes = &buscfg->bus.ccp2.lanecfg;
-- 
1.7.10.4

Re: [PATCH 14/15] omap3isp: Add support for the Device Tree

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Monday 16 March 2015 02:26:09 Sakari Ailus wrote:
> Add the ISP device to omap3 DT include file and add support to the driver to
> use it.
> 
> Also obtain information on the external entities and the ISP configuration
> related to them through the Device Tree in addition to the platform data.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> ---
>  drivers/media/platform/omap3isp/isp.c       |  209 ++++++++++++++++++++++--
>  drivers/media/platform/omap3isp/isp.h       |   11 ++
>  drivers/media/platform/omap3isp/ispcsiphy.c |    7 +
>  3 files changed, 215 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/media/platform/omap3isp/isp.c
> b/drivers/media/platform/omap3isp/isp.c index 992e74c..0d6012a 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c
> @@ -64,6 +64,7 @@
> 
>  #include <media/v4l2-common.h>
>  #include <media/v4l2-device.h>
> +#include <media/v4l2-of.h>
> 
>  #include "isp.h"
>  #include "ispreg.h"
> @@ -1991,6 +1992,14 @@ static int isp_register_entities(struct isp_device
> *isp) if (ret < 0)
>  		goto done;
> 
> +	/*
> +	 * Device Tree --- the external of the sub-devices will be

What do you mean by "the external of the sub-devices" ?

> +	 * registered later. Same goes for the sub-device node
> +	 * registration.
> +	 */
> +	if (isp->dev->of_node)
> +		return 0;
> +
>  	/* Register external entities */
>  	for (isp_subdev = pdata ? pdata->subdevs : NULL;
>  	     isp_subdev && isp_subdev->board_info; isp_subdev++) {
> @@ -2016,8 +2025,10 @@ static int isp_register_entities(struct isp_device
> *isp) ret = v4l2_device_register_subdev_nodes(&isp->v4l2_dev);
> 
>  done:
> -	if (ret < 0)
> +	if (ret < 0) {
>  		isp_unregister_entities(isp);
> +		v4l2_async_notifier_unregister(&isp->notifier);
> +	}
> 
>  	return ret;
>  }
> @@ -2232,6 +2243,7 @@ static int isp_remove(struct platform_device *pdev)
>  {
>  	struct isp_device *isp = platform_get_drvdata(pdev);
> 
> +	v4l2_async_notifier_unregister(&isp->notifier);
>  	isp_unregister_entities(isp);
>  	isp_cleanup_modules(isp);
>  	isp_xclk_cleanup(isp);
> @@ -2243,6 +2255,151 @@ static int isp_remove(struct platform_device *pdev)
>  	return 0;
>  }
> 
> +enum isp_of_phy {
> +	ISP_OF_PHY_PARALLEL = 0,
> +	ISP_OF_PHY_CSIPHY1,
> +	ISP_OF_PHY_CSIPHY2,
> +};
> +
> +static int isp_of_parse_node(struct device *dev, struct device_node *node,
> +			     struct isp_async_subdev *isd)
> +{
> +	struct isp_bus_cfg *buscfg = &isd->bus;
> +	struct v4l2_of_endpoint vep;
> +	unsigned int i;
> +
> +	v4l2_of_parse_endpoint(node, &vep);
> +
> +	dev_dbg(dev, "interface %u\n", vep.base.port);

The message seems a bit terse to me, I would also print the endpoint node name 
to give a bit of context.

	dev_dbg(dev, "parsing endpoint %s, interface %u\n", node->full_name,
		vep.base.port);

> +
> +	switch (vep.base.port) {
> +	case ISP_OF_PHY_PARALLEL:
> +		buscfg->interface = ISP_INTERFACE_PARALLEL;
> +		buscfg->bus.parallel.data_lane_shift =
> +			vep.bus.parallel.data_shift;
> +		buscfg->bus.parallel.clk_pol =
> +			!!(vep.bus.parallel.flags
> +			   & V4L2_MBUS_PCLK_SAMPLE_FALLING);
> +		buscfg->bus.parallel.hs_pol =
> +			!!(vep.bus.parallel.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW);
> +		buscfg->bus.parallel.vs_pol =
> +			!!(vep.bus.parallel.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW);
> +		buscfg->bus.parallel.fld_pol =
> +			!!(vep.bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
> +		buscfg->bus.parallel.data_pol =
> +			!!(vep.bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
> +		break;
> +
> +	case ISP_OF_PHY_CSIPHY1:
> +	case ISP_OF_PHY_CSIPHY2:
> +		/* FIXME: always assume CSI-2 for now. */
> +		switch (vep.base.port) {
> +		case ISP_OF_PHY_CSIPHY1:

I'd use an if - else here, but that's up to you.

> +			buscfg->interface = ISP_INTERFACE_CSI2C_PHY1;
> +			break;
> +		case ISP_OF_PHY_CSIPHY2:
> +			buscfg->interface = ISP_INTERFACE_CSI2A_PHY2;
> +			break;
> +		}
> +		buscfg->bus.csi2.lanecfg.clk.pos = vep.bus.mipi_csi2.clock_lane;
> +		buscfg->bus.csi2.lanecfg.clk.pol =
> +			vep.bus.mipi_csi2.lane_polarity[0];
> +		dev_dbg(dev, "clock lane polarity %u, pos %u\n",
> +			buscfg->bus.csi2.lanecfg.clk.pol,
> +			buscfg->bus.csi2.lanecfg.clk.pos);
> +
> +		for (i = 0; i < ISP_CSIPHY2_NUM_DATA_LANES; i++) {
> +			buscfg->bus.csi2.lanecfg.data[i].pos =
> +				vep.bus.mipi_csi2.data_lanes[i];
> +			buscfg->bus.csi2.lanecfg.data[i].pol =
> +				vep.bus.mipi_csi2.lane_polarity[i + 1];
> +			dev_dbg(dev, "data lane %u polarity %u, pos %u\n", i,
> +				buscfg->bus.csi2.lanecfg.data[i].pol,
> +				buscfg->bus.csi2.lanecfg.data[i].pos);
> +		}
> +
> +		/*
> +		 * FIXME: now we assume the CRC is always there.
> +		 * Implement a way to obtain this information from the
> +		 * sensor. Frame descriptors, perhaps?
> +		 */
> +		buscfg->bus.csi2.crc = 1;
> +		break;
> +
> +	default:
> +		dev_warn(dev, "invalid interface %d\n\n", vep.base.port);

Double \n ? I would also print the node name to add a bit of context.

> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +static int isp_of_parse_nodes(struct device *dev,
> +			      struct v4l2_async_notifier *notifier)
> +{
> +	struct device_node *node = NULL;

No need to initialize node to NULL.

> +	struct v4l2_async_subdev **asds;
> +
> +	asds = notifier->subdevs =

Could you use a single assignment per line, please ?

> +		devm_kcalloc(dev, ISP_MAX_SUBDEVS, sizeof(*notifier->subdevs),
> +			     GFP_KERNEL);
> +	if (!asds)
> +		return -ENOMEM;
> +
> +	while ((node = of_graph_get_next_endpoint(dev->of_node, node)) &&
> +	       notifier->num_subdevs < ISP_MAX_SUBDEVS) {
> +		struct isp_async_subdev *isd;
> +
> +		isd = devm_kzalloc(dev, sizeof(*isd), GFP_KERNEL);
> +		if (!isd)
> +			return -ENOMEM;
> +
> +		*asds = &isd->asd;
> +
> +		if (isp_of_parse_node(dev, node, isd))
> +			return -EINVAL;
> +
> +		isd->asd.match.of.node = of_graph_get_remote_port_parent(node);

An of_node_put() is needed somewhere in the exit/cleanup paths.

> +
> +		if (!isd->asd.match.of.node) {
> +			dev_warn(dev, "bad remote port parent\n");
> +			return -EINVAL;
> +		}
> +		isd->asd.match_type = V4L2_ASYNC_MATCH_OF;
> +		notifier->num_subdevs++;
> +	}
> +
> +	return notifier->num_subdevs;
> +}
> +
> +static int isp_subdev_notifier_bound(struct v4l2_async_notifier *async,
> +				     struct v4l2_subdev *subdev,
> +				     struct v4l2_async_subdev *asd)
> +{
> +	struct isp_device *isp = container_of(async, struct isp_device,
> +					      notifier);
> +	struct isp_async_subdev *isd =
> +		container_of(asd, struct isp_async_subdev, asd);
> +	int rval;

The coding style in the omap3isp driver mostly uses ret, sorry :-)

> +
> +	rval = isp_link_entity(isp, &subdev->entity, isd->bus.interface);
> +	if (rval < 0)
> +		return rval;
> +
> +	isd->sd = subdev;
> +	isd->sd->host_priv = &isd->bus;
> +
> +	return rval;
> +}
> +
> +static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
> +{
> +	struct isp_device *isp = container_of(async, struct isp_device,
> +					      notifier);
> +
> +	return v4l2_device_register_subdev_nodes(&isp->v4l2_dev);
> +}
> +
>  /*
>   * isp_probe - Probe ISP platform device
>   * @pdev: Pointer to ISP platform device
> @@ -2256,7 +2413,6 @@ static int isp_remove(struct platform_device *pdev)
>   */
>  static int isp_probe(struct platform_device *pdev)
>  {
> -	struct isp_platform_data *pdata = pdev->dev.platform_data;
>  	struct isp_device *isp;
>  	struct resource *mem;
>  	int ret;
> @@ -2268,13 +2424,37 @@ static int isp_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  	}
> 
> +	if (IS_ENABLED(CONFIG_OF) && pdev->dev.of_node) {
> +		ret = of_property_read_u32(pdev->dev.of_node, "ti,phy-type",
> +					   &isp->phy_type);
> +		if (ret)
> +			return ret;
> +
> +		isp->syscon = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
> +							      "syscon");
> +		if (IS_ERR(isp->syscon))
> +			return PTR_ERR(isp->syscon);

isp->syscon_offset isn't set anywhere in the DT case, am I missing something ?

> +
> +		ret = isp_of_parse_nodes(&pdev->dev, &isp->notifier);
> +		if (ret < 0)
> +			return ret;
> +		ret = v4l2_async_notifier_register(&isp->v4l2_dev,
> +						   &isp->notifier);
> +		if (ret)
> +			return ret;
> +	} else {
> +		isp->pdata = pdev->dev.platform_data;
> +		isp->syscon = syscon_regmap_lookup_by_pdevname("syscon.0");
> +		if (IS_ERR(isp->syscon))
> +			return PTR_ERR(isp->syscon);
> +	}
> +
>  	isp->autoidle = autoidle;
> 
>  	mutex_init(&isp->isp_mutex);
>  	spin_lock_init(&isp->stat_lock);
> 
>  	isp->dev = &pdev->dev;
> -	isp->pdata = pdata;
>  	isp->ref_count = 0;
> 
>  	ret = dma_coerce_mask_and_coherent(isp->dev, DMA_BIT_MASK(32));
> @@ -2346,6 +2526,11 @@ static int isp_probe(struct platform_device *pdev)
>  		goto error_isp;
>  	}
> 
> +	if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node) {
> +		isp->syscon_offset = isp_res_maps[m].syscon_offset;
> +		isp->phy_type = isp_res_maps[m].phy_type;
> +	}
> +
>  	for (i = 1; i < OMAP3_ISP_IOMEM_CSI2A_REGS1; i++)
>  		isp->mmio_base[i] =
>  			isp->mmio_base[0] + isp_res_maps[m].offset[i];
> @@ -2358,15 +2543,6 @@ static int isp_probe(struct platform_device *pdev)
>  	isp->mmio_hist_base_phys =
>  		mem->start + isp_res_maps[m].offset[OMAP3_ISP_IOMEM_HIST];
> 
> -	isp->syscon = syscon_regmap_lookup_by_pdevname("syscon.0");
> -	if (IS_ERR(isp->syscon)) {
> -		ret = PTR_ERR(isp->syscon);
> -		goto error_isp;
> -	}
> -
> -	isp->syscon_offset = isp_res_maps[m].syscon_offset;
> -	isp->phy_type = isp_res_maps[m].phy_type;
> -
>  	/* IOMMU */
>  	ret = isp_attach_iommu(isp);
>  	if (ret < 0) {
> @@ -2394,6 +2570,9 @@ static int isp_probe(struct platform_device *pdev)
>  	if (ret < 0)
>  		goto error_iommu;
> 
> +	isp->notifier.bound = isp_subdev_notifier_bound;
> +	isp->notifier.complete = isp_subdev_notifier_complete;
> +
>  	ret = isp_register_entities(isp);
>  	if (ret < 0)
>  		goto error_modules;
> @@ -2429,6 +2608,11 @@ static struct platform_device_id omap3isp_id_table[]
> = { };
>  MODULE_DEVICE_TABLE(platform, omap3isp_id_table);
> 
> +static const struct of_device_id omap3isp_of_table[] = {
> +	{ .compatible = "ti,omap3-isp" },
> +	{ },
> +};
> +
>  static struct platform_driver omap3isp_driver = {
>  	.probe = isp_probe,
>  	.remove = isp_remove,
> @@ -2436,6 +2620,7 @@ static struct platform_driver omap3isp_driver = {
>  	.driver = {
>  		.name = "omap3isp",
>  		.pm	= &omap3isp_pm_ops,
> +		.of_match_table = omap3isp_of_table,
>  	},
>  };
> 
> diff --git a/drivers/media/platform/omap3isp/isp.h
> b/drivers/media/platform/omap3isp/isp.h index dcb7d20..431224e 100644
> --- a/drivers/media/platform/omap3isp/isp.h
> +++ b/drivers/media/platform/omap3isp/isp.h
> @@ -18,6 +18,7 @@
>  #define OMAP3_ISP_CORE_H
> 
>  #include <media/omap3isp.h>
> +#include <media/v4l2-async.h>
>  #include <media/v4l2-device.h>
>  #include <linux/clk-provider.h>
>  #include <linux/device.h>
> @@ -178,6 +179,7 @@ struct isp_xclk {
>   */
>  struct isp_device {
>  	struct v4l2_device v4l2_dev;
> +	struct v4l2_async_notifier notifier;
>  	struct media_device media_dev;
>  	struct device *dev;
>  	u32 revision;
> @@ -224,6 +226,15 @@ struct isp_device {
> 
>  	unsigned int sbl_resources;
>  	unsigned int subclk_resources;
> +
> +#define ISP_MAX_SUBDEVS		8
> +	struct v4l2_subdev *subdevs[ISP_MAX_SUBDEVS];
> +};
> +
> +struct isp_async_subdev {
> +	struct v4l2_subdev *sd;
> +	struct isp_bus_cfg bus;
> +	struct v4l2_async_subdev asd;
>  };
> 
>  #define v4l2_dev_to_isp_device(dev) \
> diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c
> b/drivers/media/platform/omap3isp/ispcsiphy.c index d91dde1..495447d 100644
> --- a/drivers/media/platform/omap3isp/ispcsiphy.c
> +++ b/drivers/media/platform/omap3isp/ispcsiphy.c
> @@ -173,6 +173,13 @@ static int omap3isp_csiphy_config(struct isp_csiphy
> *phy) unsigned int i;
>  	u32 reg;
> 
> +	if (!buscfg) {
> +		struct isp_async_subdev *isd =
> +			container_of(pipe->external->asd,
> +				     struct isp_async_subdev, asd);
> +		buscfg = &isd->bus;
> +	}
> +
>  	if (buscfg->interface == ISP_INTERFACE_CCP2B_PHY1
> 
>  	    || buscfg->interface == ISP_INTERFACE_CCP2B_PHY2)
> 
>  		lanes = &buscfg->bus.ccp2.lanecfg;

-- 
Regards,

Laurent Pinchart

Re: [PATCH 14/15] omap3isp: Add support for the Device Tree

[Sakari Ailus]

Hi Laurent,

Thanks for the review.

On Tue, Mar 17, 2015 at 02:48:54AM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Monday 16 March 2015 02:26:09 Sakari Ailus wrote:
> > Add the ISP device to omap3 DT include file and add support to the driver to
> > use it.
> > 
> > Also obtain information on the external entities and the ISP configuration
> > related to them through the Device Tree in addition to the platform data.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> > ---
> >  drivers/media/platform/omap3isp/isp.c       |  209 ++++++++++++++++++++++--
> >  drivers/media/platform/omap3isp/isp.h       |   11 ++
> >  drivers/media/platform/omap3isp/ispcsiphy.c |    7 +
> >  3 files changed, 215 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/media/platform/omap3isp/isp.c
> > b/drivers/media/platform/omap3isp/isp.c index 992e74c..0d6012a 100644
> > --- a/drivers/media/platform/omap3isp/isp.c
> > +++ b/drivers/media/platform/omap3isp/isp.c
> > @@ -64,6 +64,7 @@
> > 
> >  #include <media/v4l2-common.h>
> >  #include <media/v4l2-device.h>
> > +#include <media/v4l2-of.h>
> > 
> >  #include "isp.h"
> >  #include "ispreg.h"
> > @@ -1991,6 +1992,14 @@ static int isp_register_entities(struct isp_device
> > *isp) if (ret < 0)
> >  		goto done;
> > 
> > +	/*
> > +	 * Device Tree --- the external of the sub-devices will be
> 
> What do you mean by "the external of the sub-devices" ?

This is probably a mind'o. I removed "of the ", and added the article to the
second sentence, so it becomes:

	/*
	 * Device Tree --- the external sub-devices will be registered
	 * later. The same goes for the sub-device node registration.
	 */


> > +	 * registered later. Same goes for the sub-device node
> > +	 * registration.
> > +	 */
> > +	if (isp->dev->of_node)
> > +		return 0;
> > +
> >  	/* Register external entities */
> >  	for (isp_subdev = pdata ? pdata->subdevs : NULL;
> >  	     isp_subdev && isp_subdev->board_info; isp_subdev++) {
> > @@ -2016,8 +2025,10 @@ static int isp_register_entities(struct isp_device
> > *isp) ret = v4l2_device_register_subdev_nodes(&isp->v4l2_dev);
> > 
> >  done:
> > -	if (ret < 0)
> > +	if (ret < 0) {
> >  		isp_unregister_entities(isp);
> > +		v4l2_async_notifier_unregister(&isp->notifier);
> > +	}
> > 
> >  	return ret;
> >  }
> > @@ -2232,6 +2243,7 @@ static int isp_remove(struct platform_device *pdev)
> >  {
> >  	struct isp_device *isp = platform_get_drvdata(pdev);
> > 
> > +	v4l2_async_notifier_unregister(&isp->notifier);
> >  	isp_unregister_entities(isp);
> >  	isp_cleanup_modules(isp);
> >  	isp_xclk_cleanup(isp);
> > @@ -2243,6 +2255,151 @@ static int isp_remove(struct platform_device *pdev)
> >  	return 0;
> >  }
> > 
> > +enum isp_of_phy {
> > +	ISP_OF_PHY_PARALLEL = 0,
> > +	ISP_OF_PHY_CSIPHY1,
> > +	ISP_OF_PHY_CSIPHY2,
> > +};
> > +
> > +static int isp_of_parse_node(struct device *dev, struct device_node *node,
> > +			     struct isp_async_subdev *isd)
> > +{
> > +	struct isp_bus_cfg *buscfg = &isd->bus;
> > +	struct v4l2_of_endpoint vep;
> > +	unsigned int i;
> > +
> > +	v4l2_of_parse_endpoint(node, &vep);
> > +
> > +	dev_dbg(dev, "interface %u\n", vep.base.port);
> 
> The message seems a bit terse to me, I would also print the endpoint node name 
> to give a bit of context.
> 
> 	dev_dbg(dev, "parsing endpoint %s, interface %u\n", node->full_name,
> 		vep.base.port);

I'll use this as-is.

> 
> > +
> > +	switch (vep.base.port) {
> > +	case ISP_OF_PHY_PARALLEL:
> > +		buscfg->interface = ISP_INTERFACE_PARALLEL;
> > +		buscfg->bus.parallel.data_lane_shift =
> > +			vep.bus.parallel.data_shift;
> > +		buscfg->bus.parallel.clk_pol =
> > +			!!(vep.bus.parallel.flags
> > +			   & V4L2_MBUS_PCLK_SAMPLE_FALLING);
> > +		buscfg->bus.parallel.hs_pol =
> > +			!!(vep.bus.parallel.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW);
> > +		buscfg->bus.parallel.vs_pol =
> > +			!!(vep.bus.parallel.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW);
> > +		buscfg->bus.parallel.fld_pol =
> > +			!!(vep.bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
> > +		buscfg->bus.parallel.data_pol =
> > +			!!(vep.bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
> > +		break;
> > +
> > +	case ISP_OF_PHY_CSIPHY1:
> > +	case ISP_OF_PHY_CSIPHY2:
> > +		/* FIXME: always assume CSI-2 for now. */
> > +		switch (vep.base.port) {
> > +		case ISP_OF_PHY_CSIPHY1:
> 
> I'd use an if - else here, but that's up to you.

I do prefer switch. I think it's easier to see that way what's done here.

> > +			buscfg->interface = ISP_INTERFACE_CSI2C_PHY1;
> > +			break;
> > +		case ISP_OF_PHY_CSIPHY2:
> > +			buscfg->interface = ISP_INTERFACE_CSI2A_PHY2;
> > +			break;
> > +		}
> > +		buscfg->bus.csi2.lanecfg.clk.pos = vep.bus.mipi_csi2.clock_lane;
> > +		buscfg->bus.csi2.lanecfg.clk.pol =
> > +			vep.bus.mipi_csi2.lane_polarity[0];
> > +		dev_dbg(dev, "clock lane polarity %u, pos %u\n",
> > +			buscfg->bus.csi2.lanecfg.clk.pol,
> > +			buscfg->bus.csi2.lanecfg.clk.pos);
> > +
> > +		for (i = 0; i < ISP_CSIPHY2_NUM_DATA_LANES; i++) {
> > +			buscfg->bus.csi2.lanecfg.data[i].pos =
> > +				vep.bus.mipi_csi2.data_lanes[i];
> > +			buscfg->bus.csi2.lanecfg.data[i].pol =
> > +				vep.bus.mipi_csi2.lane_polarity[i + 1];
> > +			dev_dbg(dev, "data lane %u polarity %u, pos %u\n", i,
> > +				buscfg->bus.csi2.lanecfg.data[i].pol,
> > +				buscfg->bus.csi2.lanecfg.data[i].pos);
> > +		}
> > +
> > +		/*
> > +		 * FIXME: now we assume the CRC is always there.
> > +		 * Implement a way to obtain this information from the
> > +		 * sensor. Frame descriptors, perhaps?
> > +		 */
> > +		buscfg->bus.csi2.crc = 1;
> > +		break;
> > +
> > +	default:
> > +		dev_warn(dev, "invalid interface %d\n\n", vep.base.port);
> 
> Double \n ? I would also print the node name to add a bit of context.

I replaced it with:

		dev_warn(dev, "%s: invalid interface %u\n", node->full_name,
			 vep.base.port);

> 
> > +		break;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int isp_of_parse_nodes(struct device *dev,
> > +			      struct v4l2_async_notifier *notifier)
> > +{
> > +	struct device_node *node = NULL;
> 
> No need to initialize node to NULL.

Fixed.

> > +	struct v4l2_async_subdev **asds;
> > +
> > +	asds = notifier->subdevs =
> 
> Could you use a single assignment per line, please ?

I know the kernel coding style shuns this, but I think in this case this is
actually better. Albeit now I think there's an option better than even that,
which is fixing the bug of not incrementing the asds pointer.

I'll resend the patch, with asds removed.

> > +		devm_kcalloc(dev, ISP_MAX_SUBDEVS, sizeof(*notifier->subdevs),
> > +			     GFP_KERNEL);
> > +	if (!asds)
> > +		return -ENOMEM;
> > +
> > +	while ((node = of_graph_get_next_endpoint(dev->of_node, node)) &&
> > +	       notifier->num_subdevs < ISP_MAX_SUBDEVS) {
> > +		struct isp_async_subdev *isd;
> > +
> > +		isd = devm_kzalloc(dev, sizeof(*isd), GFP_KERNEL);
> > +		if (!isd)
> > +			return -ENOMEM;
> > +
> > +		*asds = &isd->asd;
> > +
> > +		if (isp_of_parse_node(dev, node, isd))
> > +			return -EINVAL;
> > +
> > +		isd->asd.match.of.node = of_graph_get_remote_port_parent(node);
> 
> An of_node_put() is needed somewhere in the exit/cleanup paths.

Added.

> > +
> > +		if (!isd->asd.match.of.node) {
> > +			dev_warn(dev, "bad remote port parent\n");
> > +			return -EINVAL;
> > +		}
> > +		isd->asd.match_type = V4L2_ASYNC_MATCH_OF;
> > +		notifier->num_subdevs++;
> > +	}
> > +
> > +	return notifier->num_subdevs;
> > +}
> > +
> > +static int isp_subdev_notifier_bound(struct v4l2_async_notifier *async,
> > +				     struct v4l2_subdev *subdev,
> > +				     struct v4l2_async_subdev *asd)
> > +{
> > +	struct isp_device *isp = container_of(async, struct isp_device,
> > +					      notifier);
> > +	struct isp_async_subdev *isd =
> > +		container_of(asd, struct isp_async_subdev, asd);
> > +	int rval;
> 
> The coding style in the omap3isp driver mostly uses ret, sorry :-)

Fixed.

> > +
> > +	rval = isp_link_entity(isp, &subdev->entity, isd->bus.interface);
> > +	if (rval < 0)
> > +		return rval;
> > +
> > +	isd->sd = subdev;
> > +	isd->sd->host_priv = &isd->bus;
> > +
> > +	return rval;
> > +}
> > +
> > +static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
> > +{
> > +	struct isp_device *isp = container_of(async, struct isp_device,
> > +					      notifier);
> > +
> > +	return v4l2_device_register_subdev_nodes(&isp->v4l2_dev);
> > +}
> > +
> >  /*
> >   * isp_probe - Probe ISP platform device
> >   * @pdev: Pointer to ISP platform device
> > @@ -2256,7 +2413,6 @@ static int isp_remove(struct platform_device *pdev)
> >   */
> >  static int isp_probe(struct platform_device *pdev)
> >  {
> > -	struct isp_platform_data *pdata = pdev->dev.platform_data;
> >  	struct isp_device *isp;
> >  	struct resource *mem;
> >  	int ret;
> > @@ -2268,13 +2424,37 @@ static int isp_probe(struct platform_device *pdev)
> >  		return -ENOMEM;
> >  	}
> > 
> > +	if (IS_ENABLED(CONFIG_OF) && pdev->dev.of_node) {
> > +		ret = of_property_read_u32(pdev->dev.of_node, "ti,phy-type",
> > +					   &isp->phy_type);
> > +		if (ret)
> > +			return ret;
> > +
> > +		isp->syscon = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
> > +							      "syscon");
> > +		if (IS_ERR(isp->syscon))
> > +			return PTR_ERR(isp->syscon);
> 
> isp->syscon_offset isn't set anywhere in the DT case, am I missing something ?

The offset is there, in the syscon specifier (phandle and optional register
offset).

> > +
> > +		ret = isp_of_parse_nodes(&pdev->dev, &isp->notifier);
> > +		if (ret < 0)
> > +			return ret;
> > +		ret = v4l2_async_notifier_register(&isp->v4l2_dev,
> > +						   &isp->notifier);
> > +		if (ret)
> > +			return ret;
> > +	} else {
> > +		isp->pdata = pdev->dev.platform_data;
> > +		isp->syscon = syscon_regmap_lookup_by_pdevname("syscon.0");
> > +		if (IS_ERR(isp->syscon))
> > +			return PTR_ERR(isp->syscon);
> > +	}
> > +
> >  	isp->autoidle = autoidle;
> > 
> >  	mutex_init(&isp->isp_mutex);
> >  	spin_lock_init(&isp->stat_lock);
> > 
> >  	isp->dev = &pdev->dev;
> > -	isp->pdata = pdata;
> >  	isp->ref_count = 0;
> > 
> >  	ret = dma_coerce_mask_and_coherent(isp->dev, DMA_BIT_MASK(32));
> > @@ -2346,6 +2526,11 @@ static int isp_probe(struct platform_device *pdev)
> >  		goto error_isp;
> >  	}
> > 
> > +	if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node) {
> > +		isp->syscon_offset = isp_res_maps[m].syscon_offset;
> > +		isp->phy_type = isp_res_maps[m].phy_type;
> > +	}
> > +
> >  	for (i = 1; i < OMAP3_ISP_IOMEM_CSI2A_REGS1; i++)
> >  		isp->mmio_base[i] =
> >  			isp->mmio_base[0] + isp_res_maps[m].offset[i];
...

-- 
Regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

[PATCH v1.1 14/15] omap3isp: Add support for the Device Tree

[Sakari Ailus]

Add the ISP device to omap3 DT include file and add support to the driver to
use it.

Also obtain information on the external entities and the ISP configuration
related to them through the Device Tree in addition to the platform data.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
---
since v1:

- Print endpoint name in debug message when parsing an endpoint.

- Rename lane-polarity property as lane-polarities.

- Print endpoint name with the interface if the interface is invalid.

- Remove assignment to two variables at the same time.

- Fix multiple sub-device support in isp_of_parse_nodes().

- Put of_node properly in isp_of_parse_nodes() (requires Philipp Zabel's
  patch "of: Decrement refcount of previous endpoint in
  of_graph_get_next_endpoint".

- Rename return value variable rval as ret to be consistent with the rest of
  the driver.

- Read the register offset from the syscom property's first argument.

 drivers/media/platform/omap3isp/isp.c       |  218 +++++++++++++++++++++++++--
 drivers/media/platform/omap3isp/isp.h       |   11 ++
 drivers/media/platform/omap3isp/ispcsiphy.c |    7 +
 3 files changed, 224 insertions(+), 12 deletions(-)

diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 992e74c..92a859e 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -64,6 +64,7 @@
 
 #include <media/v4l2-common.h>
 #include <media/v4l2-device.h>
+#include <media/v4l2-of.h>
 
 #include "isp.h"
 #include "ispreg.h"
@@ -1991,6 +1992,13 @@ static int isp_register_entities(struct isp_device *isp)
 	if (ret < 0)
 		goto done;
 
+	/*
+	 * Device Tree --- the external sub-devices will be registered
+	 * later. The same goes for the sub-device node registration.
+	 */
+	if (isp->dev->of_node)
+		return 0;
+
 	/* Register external entities */
 	for (isp_subdev = pdata ? pdata->subdevs : NULL;
 	     isp_subdev && isp_subdev->board_info; isp_subdev++) {
@@ -2016,8 +2024,10 @@ static int isp_register_entities(struct isp_device *isp)
 	ret = v4l2_device_register_subdev_nodes(&isp->v4l2_dev);
 
 done:
-	if (ret < 0)
+	if (ret < 0) {
 		isp_unregister_entities(isp);
+		v4l2_async_notifier_unregister(&isp->notifier);
+	}
 
 	return ret;
 }
@@ -2232,6 +2242,7 @@ static int isp_remove(struct platform_device *pdev)
 {
 	struct isp_device *isp = platform_get_drvdata(pdev);
 
+	v4l2_async_notifier_unregister(&isp->notifier);
 	isp_unregister_entities(isp);
 	isp_cleanup_modules(isp);
 	isp_xclk_cleanup(isp);
@@ -2243,6 +2254,156 @@ static int isp_remove(struct platform_device *pdev)
 	return 0;
 }
 
+enum isp_of_phy {
+	ISP_OF_PHY_PARALLEL = 0,
+	ISP_OF_PHY_CSIPHY1,
+	ISP_OF_PHY_CSIPHY2,
+};
+
+static int isp_of_parse_node(struct device *dev, struct device_node *node,
+			     struct isp_async_subdev *isd)
+{
+	struct isp_bus_cfg *buscfg = &isd->bus;
+	struct v4l2_of_endpoint vep;
+	unsigned int i;
+
+	v4l2_of_parse_endpoint(node, &vep);
+
+	dev_dbg(dev, "parsing endpoint %s, interface %u\n", node->full_name,
+		vep.base.port);
+
+	switch (vep.base.port) {
+	case ISP_OF_PHY_PARALLEL:
+		buscfg->interface = ISP_INTERFACE_PARALLEL;
+		buscfg->bus.parallel.data_lane_shift =
+			vep.bus.parallel.data_shift;
+		buscfg->bus.parallel.clk_pol =
+			!!(vep.bus.parallel.flags
+			   & V4L2_MBUS_PCLK_SAMPLE_FALLING);
+		buscfg->bus.parallel.hs_pol =
+			!!(vep.bus.parallel.flags & V4L2_MBUS_VSYNC_ACTIVE_LOW);
+		buscfg->bus.parallel.vs_pol =
+			!!(vep.bus.parallel.flags & V4L2_MBUS_HSYNC_ACTIVE_LOW);
+		buscfg->bus.parallel.fld_pol =
+			!!(vep.bus.parallel.flags & V4L2_MBUS_FIELD_EVEN_LOW);
+		buscfg->bus.parallel.data_pol =
+			!!(vep.bus.parallel.flags & V4L2_MBUS_DATA_ACTIVE_LOW);
+		break;
+
+	case ISP_OF_PHY_CSIPHY1:
+	case ISP_OF_PHY_CSIPHY2:
+		/* FIXME: always assume CSI-2 for now. */
+		switch (vep.base.port) {
+		case ISP_OF_PHY_CSIPHY1:
+			buscfg->interface = ISP_INTERFACE_CSI2C_PHY1;
+			break;
+		case ISP_OF_PHY_CSIPHY2:
+			buscfg->interface = ISP_INTERFACE_CSI2A_PHY2;
+			break;
+		}
+		buscfg->bus.csi2.lanecfg.clk.pos = vep.bus.mipi_csi2.clock_lane;
+		buscfg->bus.csi2.lanecfg.clk.pol =
+			vep.bus.mipi_csi2.lane_polarities[0];
+		dev_dbg(dev, "clock lane polarity %u, pos %u\n",
+			buscfg->bus.csi2.lanecfg.clk.pol,
+			buscfg->bus.csi2.lanecfg.clk.pos);
+
+		for (i = 0; i < ISP_CSIPHY2_NUM_DATA_LANES; i++) {
+			buscfg->bus.csi2.lanecfg.data[i].pos =
+				vep.bus.mipi_csi2.data_lanes[i];
+			buscfg->bus.csi2.lanecfg.data[i].pol =
+				vep.bus.mipi_csi2.lane_polarities[i + 1];
+			dev_dbg(dev, "data lane %u polarity %u, pos %u\n", i,
+				buscfg->bus.csi2.lanecfg.data[i].pol,
+				buscfg->bus.csi2.lanecfg.data[i].pos);
+		}
+
+		/*
+		 * FIXME: now we assume the CRC is always there.
+		 * Implement a way to obtain this information from the
+		 * sensor. Frame descriptors, perhaps?
+		 */
+		buscfg->bus.csi2.crc = 1;
+		break;
+
+	default:
+		dev_warn(dev, "%s: invalid interface %u\n", node->full_name,
+			 vep.base.port);
+		break;
+	}
+
+	return 0;
+}
+
+static int isp_of_parse_nodes(struct device *dev,
+			      struct v4l2_async_notifier *notifier)
+{
+	struct device_node *node;
+
+	notifier->subdevs = devm_kcalloc(
+		dev, ISP_MAX_SUBDEVS, sizeof(*notifier->subdevs), GFP_KERNEL);
+	if (!notifier->subdevs)
+		return -ENOMEM;
+
+	while ((node = of_graph_get_next_endpoint(dev->of_node, node)) &&
+	       notifier->num_subdevs < ISP_MAX_SUBDEVS) {
+		struct isp_async_subdev *isd;
+
+		isd = devm_kzalloc(dev, sizeof(*isd), GFP_KERNEL);
+		if (!isd) {
+			of_node_put(node);
+			return -ENOMEM;
+		}
+
+		notifier->subdevs[notifier->num_subdevs] = &isd->asd;
+
+		if (isp_of_parse_node(dev, node, isd)) {
+			of_node_put(node);
+			return -EINVAL;
+		}
+
+		isd->asd.match.of.node = of_graph_get_remote_port_parent(node);
+		of_node_put(node);
+		if (!isd->asd.match.of.node) {
+			dev_warn(dev, "bad remote port parent\n");
+			return -EINVAL;
+		}
+
+		isd->asd.match_type = V4L2_ASYNC_MATCH_OF;
+		notifier->num_subdevs++;
+	}
+
+	return notifier->num_subdevs;
+}
+
+static int isp_subdev_notifier_bound(struct v4l2_async_notifier *async,
+				     struct v4l2_subdev *subdev,
+				     struct v4l2_async_subdev *asd)
+{
+	struct isp_device *isp = container_of(async, struct isp_device,
+					      notifier);
+	struct isp_async_subdev *isd =
+		container_of(asd, struct isp_async_subdev, asd);
+	int ret;
+
+	ret = isp_link_entity(isp, &subdev->entity, isd->bus.interface);
+	if (ret < 0)
+		return ret;
+
+	isd->sd = subdev;
+	isd->sd->host_priv = &isd->bus;
+
+	return ret;
+}
+
+static int isp_subdev_notifier_complete(struct v4l2_async_notifier *async)
+{
+	struct isp_device *isp = container_of(async, struct isp_device,
+					      notifier);
+
+	return v4l2_device_register_subdev_nodes(&isp->v4l2_dev);
+}
+
 /*
  * isp_probe - Probe ISP platform device
  * @pdev: Pointer to ISP platform device
@@ -2256,7 +2417,6 @@ static int isp_remove(struct platform_device *pdev)
  */
 static int isp_probe(struct platform_device *pdev)
 {
-	struct isp_platform_data *pdata = pdev->dev.platform_data;
 	struct isp_device *isp;
 	struct resource *mem;
 	int ret;
@@ -2268,13 +2428,42 @@ static int isp_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	if (IS_ENABLED(CONFIG_OF) && pdev->dev.of_node) {
+		ret = of_property_read_u32(pdev->dev.of_node, "ti,phy-type",
+					   &isp->phy_type);
+		if (ret)
+			return ret;
+
+		isp->syscon = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
+							      "syscon");
+		if (IS_ERR(isp->syscon))
+			return PTR_ERR(isp->syscon);
+
+		ret = of_property_read_u32_index(pdev->dev.of_node, "syscon", 1,
+						 &isp->syscon_offset);
+		if (ret)
+			return ret;
+
+		ret = isp_of_parse_nodes(&pdev->dev, &isp->notifier);
+		if (ret < 0)
+			return ret;
+		ret = v4l2_async_notifier_register(&isp->v4l2_dev,
+						   &isp->notifier);
+		if (ret)
+			return ret;
+	} else {
+		isp->pdata = pdev->dev.platform_data;
+		isp->syscon = syscon_regmap_lookup_by_pdevname("syscon.0");
+		if (IS_ERR(isp->syscon))
+			return PTR_ERR(isp->syscon);
+	}
+
 	isp->autoidle = autoidle;
 
 	mutex_init(&isp->isp_mutex);
 	spin_lock_init(&isp->stat_lock);
 
 	isp->dev = &pdev->dev;
-	isp->pdata = pdata;
 	isp->ref_count = 0;
 
 	ret = dma_coerce_mask_and_coherent(isp->dev, DMA_BIT_MASK(32));
@@ -2346,6 +2535,11 @@ static int isp_probe(struct platform_device *pdev)
 		goto error_isp;
 	}
 
+	if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node) {
+		isp->syscon_offset = isp_res_maps[m].syscon_offset;
+		isp->phy_type = isp_res_maps[m].phy_type;
+	}
+
 	for (i = 1; i < OMAP3_ISP_IOMEM_CSI2A_REGS1; i++)
 		isp->mmio_base[i] =
 			isp->mmio_base[0] + isp_res_maps[m].offset[i];
@@ -2358,15 +2552,6 @@ static int isp_probe(struct platform_device *pdev)
 	isp->mmio_hist_base_phys =
 		mem->start + isp_res_maps[m].offset[OMAP3_ISP_IOMEM_HIST];
 
-	isp->syscon = syscon_regmap_lookup_by_pdevname("syscon.0");
-	if (IS_ERR(isp->syscon)) {
-		ret = PTR_ERR(isp->syscon);
-		goto error_isp;
-	}
-
-	isp->syscon_offset = isp_res_maps[m].syscon_offset;
-	isp->phy_type = isp_res_maps[m].phy_type;
-
 	/* IOMMU */
 	ret = isp_attach_iommu(isp);
 	if (ret < 0) {
@@ -2394,6 +2579,9 @@ static int isp_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto error_iommu;
 
+	isp->notifier.bound = isp_subdev_notifier_bound;
+	isp->notifier.complete = isp_subdev_notifier_complete;
+
 	ret = isp_register_entities(isp);
 	if (ret < 0)
 		goto error_modules;
@@ -2429,6 +2617,11 @@ static struct platform_device_id omap3isp_id_table[] = {
 };
 MODULE_DEVICE_TABLE(platform, omap3isp_id_table);
 
+static const struct of_device_id omap3isp_of_table[] = {
+	{ .compatible = "ti,omap3-isp" },
+	{ },
+};
+
 static struct platform_driver omap3isp_driver = {
 	.probe = isp_probe,
 	.remove = isp_remove,
@@ -2436,6 +2629,7 @@ static struct platform_driver omap3isp_driver = {
 	.driver = {
 		.name = "omap3isp",
 		.pm	= &omap3isp_pm_ops,
+		.of_match_table = omap3isp_of_table,
 	},
 };
 
diff --git a/drivers/media/platform/omap3isp/isp.h b/drivers/media/platform/omap3isp/isp.h
index dcb7d20..431224e 100644
--- a/drivers/media/platform/omap3isp/isp.h
+++ b/drivers/media/platform/omap3isp/isp.h
@@ -18,6 +18,7 @@
 #define OMAP3_ISP_CORE_H
 
 #include <media/omap3isp.h>
+#include <media/v4l2-async.h>
 #include <media/v4l2-device.h>
 #include <linux/clk-provider.h>
 #include <linux/device.h>
@@ -178,6 +179,7 @@ struct isp_xclk {
  */
 struct isp_device {
 	struct v4l2_device v4l2_dev;
+	struct v4l2_async_notifier notifier;
 	struct media_device media_dev;
 	struct device *dev;
 	u32 revision;
@@ -224,6 +226,15 @@ struct isp_device {
 
 	unsigned int sbl_resources;
 	unsigned int subclk_resources;
+
+#define ISP_MAX_SUBDEVS		8
+	struct v4l2_subdev *subdevs[ISP_MAX_SUBDEVS];
+};
+
+struct isp_async_subdev {
+	struct v4l2_subdev *sd;
+	struct isp_bus_cfg bus;
+	struct v4l2_async_subdev asd;
 };
 
 #define v4l2_dev_to_isp_device(dev) \
diff --git a/drivers/media/platform/omap3isp/ispcsiphy.c b/drivers/media/platform/omap3isp/ispcsiphy.c
index d91dde1..495447d 100644
--- a/drivers/media/platform/omap3isp/ispcsiphy.c
+++ b/drivers/media/platform/omap3isp/ispcsiphy.c
@@ -173,6 +173,13 @@ static int omap3isp_csiphy_config(struct isp_csiphy *phy)
 	unsigned int i;
 	u32 reg;
 
+	if (!buscfg) {
+		struct isp_async_subdev *isd =
+			container_of(pipe->external->asd,
+				     struct isp_async_subdev, asd);
+		buscfg = &isd->bus;
+	}
+
 	if (buscfg->interface == ISP_INTERFACE_CCP2B_PHY1
 	    || buscfg->interface == ISP_INTERFACE_CCP2B_PHY2)
 		lanes = &buscfg->bus.ccp2.lanecfg;
-- 
1.7.10.4

Re: [PATCH v1.1 14/15] omap3isp: Add support for the Device Tree

[Sakari Ailus]

Hi Laurent,

On Sat, Mar 21, 2015 at 12:05:04AM +0200, Sakari Ailus wrote:
> Add the ISP device to omap3 DT include file and add support to the driver to
> use it.
> 
> Also obtain information on the external entities and the ISP configuration
> related to them through the Device Tree in addition to the platform data.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>

If you're happy with this version, please add the set to your tree. The
patches can be also found in here, on top of your histogram DMA changes:

	ssh://linuxtv.org/git/sailus/media_tree.git rm696-054-media

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

Re: [PATCH v1.1 14/15] omap3isp: Add support for the Device Tree

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch. This looks good to me, except that there's one last 
bug I've spotted. Please see below.

On Saturday 21 March 2015 00:05:04 Sakari Ailus wrote:
> Add the ISP device to omap3 DT include file and add support to the driver to
> use it.
> 
> Also obtain information on the external entities and the ISP configuration
> related to them through the Device Tree in addition to the platform data.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> ---
> since v1:
> 
> - Print endpoint name in debug message when parsing an endpoint.
> 
> - Rename lane-polarity property as lane-polarities.
> 
> - Print endpoint name with the interface if the interface is invalid.
> 
> - Remove assignment to two variables at the same time.
> 
> - Fix multiple sub-device support in isp_of_parse_nodes().
> 
> - Put of_node properly in isp_of_parse_nodes() (requires Philipp Zabel's
>   patch "of: Decrement refcount of previous endpoint in
>   of_graph_get_next_endpoint".
> 
> - Rename return value variable rval as ret to be consistent with the rest of
> the driver.
> 
> - Read the register offset from the syscom property's first argument.
> 
>  drivers/media/platform/omap3isp/isp.c       |  218 ++++++++++++++++++++++--
>  drivers/media/platform/omap3isp/isp.h       |   11 ++
>  drivers/media/platform/omap3isp/ispcsiphy.c |    7 +
>  3 files changed, 224 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/media/platform/omap3isp/isp.c
> b/drivers/media/platform/omap3isp/isp.c index 992e74c..92a859e 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c

[snip]

> +static int isp_of_parse_nodes(struct device *dev,
> +			      struct v4l2_async_notifier *notifier)
> +{
> +	struct device_node *node;
> +
> +	notifier->subdevs = devm_kcalloc(
> +		dev, ISP_MAX_SUBDEVS, sizeof(*notifier->subdevs), GFP_KERNEL);
> +	if (!notifier->subdevs)
> +		return -ENOMEM;
> +
> +	while ((node = of_graph_get_next_endpoint(dev->of_node, node)) &&
> +	       notifier->num_subdevs < ISP_MAX_SUBDEVS) {

If the first condition evaluates to true and the second one to false, the loop 
will be exited without releasing the reference to the DT node. You could just 
switch the two conditions to fix this.

> +		struct isp_async_subdev *isd;
> +
> +		isd = devm_kzalloc(dev, sizeof(*isd), GFP_KERNEL);
> +		if (!isd) {
> +			of_node_put(node);
> +			return -ENOMEM;
> +		}
> +
> +		notifier->subdevs[notifier->num_subdevs] = &isd->asd;
> +
> +		if (isp_of_parse_node(dev, node, isd)) {
> +			of_node_put(node);
> +			return -EINVAL;
> +		}
> +
> +		isd->asd.match.of.node = of_graph_get_remote_port_parent(node);
> +		of_node_put(node);
> +		if (!isd->asd.match.of.node) {
> +			dev_warn(dev, "bad remote port parent\n");
> +			return -EINVAL;
> +		}
> +
> +		isd->asd.match_type = V4L2_ASYNC_MATCH_OF;
> +		notifier->num_subdevs++;
> +	}
> +
> +	return notifier->num_subdevs;
> +}

-- 
Regards,

Laurent Pinchart

Re: [PATCH v1.1 14/15] omap3isp: Add support for the Device Tree

[Sakari Ailus]

Hi Laurent,

Thanks again for the comments!

On Sun, Mar 22, 2015 at 10:26:39PM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch. This looks good to me, except that there's one last 
> bug I've spotted. Please see below.
> 
> On Saturday 21 March 2015 00:05:04 Sakari Ailus wrote:
> > Add the ISP device to omap3 DT include file and add support to the driver to
> > use it.
> > 
> > Also obtain information on the external entities and the ISP configuration
> > related to them through the Device Tree in addition to the platform data.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> > ---
> > since v1:
> > 
> > - Print endpoint name in debug message when parsing an endpoint.
> > 
> > - Rename lane-polarity property as lane-polarities.
> > 
> > - Print endpoint name with the interface if the interface is invalid.
> > 
> > - Remove assignment to two variables at the same time.
> > 
> > - Fix multiple sub-device support in isp_of_parse_nodes().
> > 
> > - Put of_node properly in isp_of_parse_nodes() (requires Philipp Zabel's
> >   patch "of: Decrement refcount of previous endpoint in
> >   of_graph_get_next_endpoint".
> > 
> > - Rename return value variable rval as ret to be consistent with the rest of
> > the driver.
> > 
> > - Read the register offset from the syscom property's first argument.
> > 
> >  drivers/media/platform/omap3isp/isp.c       |  218 ++++++++++++++++++++++--
> >  drivers/media/platform/omap3isp/isp.h       |   11 ++
> >  drivers/media/platform/omap3isp/ispcsiphy.c |    7 +
> >  3 files changed, 224 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/media/platform/omap3isp/isp.c
> > b/drivers/media/platform/omap3isp/isp.c index 992e74c..92a859e 100644
> > --- a/drivers/media/platform/omap3isp/isp.c
> > +++ b/drivers/media/platform/omap3isp/isp.c
> 
> [snip]
> 
> > +static int isp_of_parse_nodes(struct device *dev,
> > +			      struct v4l2_async_notifier *notifier)
> > +{
> > +	struct device_node *node;
> > +
> > +	notifier->subdevs = devm_kcalloc(
> > +		dev, ISP_MAX_SUBDEVS, sizeof(*notifier->subdevs), GFP_KERNEL);
> > +	if (!notifier->subdevs)
> > +		return -ENOMEM;
> > +
> > +	while ((node = of_graph_get_next_endpoint(dev->of_node, node)) &&
> > +	       notifier->num_subdevs < ISP_MAX_SUBDEVS) {
> 
> If the first condition evaluates to true and the second one to false, the loop 
> will be exited without releasing the reference to the DT node. You could just 
> switch the two conditions to fix this.

Oh, I missed this. I'll resend the set.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

[PATCH 15/15] omap3isp: Deprecate platform data support

[Sakari Ailus]

Print a warning when the driver is used with platform data. Existing
platform data user should move to DT now.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
---
 drivers/media/platform/omap3isp/isp.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 0d6012a..82940fe 100644
--- a/drivers/media/platform/omap3isp/isp.c
+++ b/drivers/media/platform/omap3isp/isp.c
@@ -2447,6 +2447,8 @@ static int isp_probe(struct platform_device *pdev)
 		isp->syscon = syscon_regmap_lookup_by_pdevname("syscon.0");
 		if (IS_ERR(isp->syscon))
 			return PTR_ERR(isp->syscon);
+		dev_warn(&pdev->dev,
+			 "Platform data support is deprecated! Please move to DT now!\n");
 	}
 
 	isp->autoidle = autoidle;
-- 
1.7.10.4

Re: [PATCH 15/15] omap3isp: Deprecate platform data support

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Monday 16 March 2015 02:26:10 Sakari Ailus wrote:
> Print a warning when the driver is used with platform data. Existing
> platform data user should move to DT now.

s/user/users/

> 
> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/media/platform/omap3isp/isp.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/media/platform/omap3isp/isp.c
> b/drivers/media/platform/omap3isp/isp.c index 0d6012a..82940fe 100644
> --- a/drivers/media/platform/omap3isp/isp.c
> +++ b/drivers/media/platform/omap3isp/isp.c
> @@ -2447,6 +2447,8 @@ static int isp_probe(struct platform_device *pdev)
>  		isp->syscon = syscon_regmap_lookup_by_pdevname("syscon.0");
>  		if (IS_ERR(isp->syscon))
>  			return PTR_ERR(isp->syscon);
> +		dev_warn(&pdev->dev,
> +			 "Platform data support is deprecated! Please move to DT now!
\n");
>  	}
> 
>  	isp->autoidle = autoidle;

-- 
Regards,

Laurent Pinchart