[RFC 00/18] Device tree support for omap3isp, N9[50] primary camera

[RFC 00/18] Device tree support for omap3isp, N9[50] primary camera

[Sakari Ailus]

Hi folks,

I've had this patchset hanging around for a long, long time, and now it's
time to send it out to linux-media.

For OMAP 3, first there are a few patches for random and tiny bugfixes and
then preparation for DT support (including compile tested rework of cm-t35
board code).

The lane-polarity DT binding is added to video-interfaces.txt for this, I
believe, is hardly specific to the OMAP 3 ISP. The same property is read by
the V4L2 OF parser as well.

ISP DT support is added for both OMAP 34xx and 36xx, but it remains tested
on 3630 only. The primary camera support is added for both N950 and N9, the
latter of which is tested. The differences are quite small between the two
in both of the cases, but still testing definitely wouldn't hurt.

The secondary camera support is still lacking primarily due to the I2C
address conflict: both of the cameras have the same I2C address. It'd also
require CCP2 bus support, which is relatively trivial to add.

Effects of Philipp Zabel's patch "of: Decrement refcount of previous
endpoint in of_graph_get_next_endpoint" aren't yet taken into account in
this series. I'll address this in the next set if changes (small I presume)
are needed.

-- 
Kind regards,
Sakari

[RFC 02/18] 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>
---
 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

Re: [RFC 02/18] omap3isp: Avoid a BUG_ON() in media_entity_create_link()

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Saturday 07 March 2015 23:40:59 Sakari Ailus wrote:
> 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;

-- 
Regards,

Laurent Pinchart

[RFC 01/18] 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-X3B1VOXEql0@public.gmane.org>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 01/18] omap3isp: Fix error handling in probe

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Saturday 07 March 2015 23:40:58 Sakari Ailus wrote:
> 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);

-- 
Regards,

Laurent Pinchart

[RFC 03/18] 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-X3B1VOXEql0@public.gmane.org>
---
 drivers/media/platform/omap3isp/isp.c |  147 +++++++++++++++++----------------
 1 file changed, 74 insertions(+), 73 deletions(-)

diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 4ab674d..a607f26 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;
@@ -1894,85 +1965,15 @@ 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;
+		struct v4l2_subdev *sensor =
+			isp_register_subdev_group(isp, subdevs->subdevs);
 
-		sensor = isp_register_subdev_group(isp, subdevs->subdevs);
 		if (sensor == NULL)
 			continue;
 
 		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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 03/18] omap3isp: Separate external link creation from platform data parsing

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Saturday 07 March 2015 23:41:00 Sakari Ailus wrote:
> 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>
> ---
>  drivers/media/platform/omap3isp/isp.c |  147  +++++++++++++++--------------
>  1 file changed, 74 insertions(+), 73 deletions(-)
> 
> diff --git a/drivers/media/platform/omap3isp/isp.c
> b/drivers/media/platform/omap3isp/isp.c index 4ab674d..a607f26 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;
> @@ -1894,85 +1965,15 @@ 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;
> +		struct v4l2_subdev *sensor =
> +			isp_register_subdev_group(isp, subdevs->subdevs);
> 
> -		sensor = isp_register_subdev_group(isp, subdevs->subdevs);

Nit-picking a bit, I'd keep the variable declaration and the function call 
separate here, as isp_register_subdev_group() is much more than an 
initializer. Apart from that,

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

>  		if (sensor == NULL)
>  			continue;
> 
>  		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;
>  	}

-- 
Regards,

Laurent Pinchart

[RFC 04/18] omap3isp: DT support for clocks

[Sakari Ailus]

From: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>

Signed-off-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
---
 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 a607f26..01356dd 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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC 05/18] 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-X3B1VOXEql0@public.gmane.org>
---
 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 01356dd..b836bc8 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 =
 			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 3fe9047..d644164 100644
--- a/drivers/media/platform/omap3isp/ispvideo.c
+++ b/drivers/media/platform/omap3isp/ispvideo.c
@@ -1022,7 +1022,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;
@@ -1104,7 +1104,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
@@ -1165,7 +1165,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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 05/18] omap3isp: Platform data could be NULL

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Saturday 07 March 2015 23:41:02 Sakari Ailus wrote:
> 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 01356dd..b836bc8 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 =
>  			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 3fe9047..d644164 100644
> --- a/drivers/media/platform/omap3isp/ispvideo.c
> +++ b/drivers/media/platform/omap3isp/ispvideo.c
> @@ -1022,7 +1022,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;
> @@ -1104,7 +1104,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
> @@ -1165,7 +1165,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);

-- 
Regards,

Laurent Pinchart

[RFC 06/18] 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-X3B1VOXEql0@public.gmane.org>
Cc: Mike Rapoport <mike-UTxiZqZC01RS1MOuV/RT9w@public.gmane.org>
Cc: Igor Grinberg <grinberg-UTxiZqZC01RS1MOuV/RT9w@public.gmane.org>
---
 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 b836bc8..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 =
-			isp_register_subdev_group(isp, subdevs->subdevs);
+	for (isp_subdev = pdata ? pdata->subdevs : NULL;
+	     isp_subdev && isp_subdev->board_info; isp_subdev++) {
+		struct v4l2_subdev *sd;
 
-		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..1c0a552 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)
+ * @buscfg: 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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 06/18] omap3isp: Refactor device configuration structs for Device Tree

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Saturday 07 March 2015 23:41:03 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>
> Cc: Mike Rapoport <mike@compulab.co.il>
> 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(-)

[snip]

> diff --git a/drivers/media/platform/omap3isp/ispccdc.c
> b/drivers/media/platform/omap3isp/ispccdc.c index 587489a..1c0a552 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)
> + * @buscfg: Parallel interface platform data (may be NULL)

The parameter is called parcfg below.

With this fixed,

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

>   * @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);

-- 
Regards,

Laurent Pinchart

[RFC 09/18] 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-X3B1VOXEql0@public.gmane.org>
---
 drivers/media/platform/omap3isp/isp.c     |    3 ++-
 drivers/media/platform/omap3isp/isp.h     |    6 +++---
 drivers/media/platform/omap3isp/isphist.c |    2 +-
 3 files changed, 6 insertions(+), 5 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 ce822c3..3bb9c4f 100644
--- a/drivers/media/platform/omap3isp/isphist.c
+++ b/drivers/media/platform/omap3isp/isphist.c
@@ -70,7 +70,7 @@ static void hist_dma_config(struct ispstat *hist)
 	hist->dma_config.sync_mode = OMAP_DMA_SYNC_ELEMENT;
 	hist->dma_config.frame_count = 1;
 	hist->dma_config.src_amode = OMAP_DMA_AMODE_CONSTANT;
-	hist->dma_config.src_start = isp->mmio_base_phys[OMAP3_ISP_IOMEM_HIST]
+	hist->dma_config.src_start = isp->mmio_hist_base_phys
 				   + ISPHIST_DATA;
 	hist->dma_config.dst_amode = OMAP_DMA_AMODE_POST_INC;
 	hist->dma_config.src_or_dst_synch = OMAP_DMA_SRC_SYNC;
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 09/18] omap3isp: Replace mmio_base_phys array with the histogram block base

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Saturday 07 March 2015 23:41:06 Sakari Ailus wrote:
> 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 |    2 +-
>  3 files changed, 6 insertions(+), 5 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 ce822c3..3bb9c4f 100644
> --- a/drivers/media/platform/omap3isp/isphist.c
> +++ b/drivers/media/platform/omap3isp/isphist.c
> @@ -70,7 +70,7 @@ static void hist_dma_config(struct ispstat *hist)
>  	hist->dma_config.sync_mode = OMAP_DMA_SYNC_ELEMENT;
>  	hist->dma_config.frame_count = 1;
>  	hist->dma_config.src_amode = OMAP_DMA_AMODE_CONSTANT;
> -	hist->dma_config.src_start = isp->mmio_base_phys[OMAP3_ISP_IOMEM_HIST]
> +	hist->dma_config.src_start = isp->mmio_hist_base_phys
>  				   + ISPHIST_DATA;
>  	hist->dma_config.dst_amode = OMAP_DMA_AMODE_POST_INC;
>  	hist->dma_config.src_or_dst_synch = OMAP_DMA_SRC_SYNC;

-- 
Regards,

Laurent Pinchart

[RFC 10/18] 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-X3B1VOXEql0@public.gmane.org>
---
 arch/arm/boot/dts/omap3.dtsi                |    2 +-
 arch/arm/mach-omap2/devices.c               |   10 ----------
 drivers/media/platform/omap3isp/isp.c       |   19 +++++++++++++++----
 drivers/media/platform/omap3isp/isp.h       |   19 +++++++++++++++++--
 drivers/media/platform/omap3isp/ispcsiphy.c |   20 +++++++++-----------
 5 files changed, 42 insertions(+), 28 deletions(-)

diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
index 01b7111..fe0b293 100644
--- a/arch/arm/boot/dts/omap3.dtsi
+++ b/arch/arm/boot/dts/omap3.dtsi
@@ -183,7 +183,7 @@
 
 		omap3_scm_general: tisyscon@48002270 {
 			compatible = "syscon";
-			reg = <0x48002270 0x2f0>;
+			reg = <0x48002270 0x2f4>;
 		};
 
 		pbias_regulator: pbias_regulator {
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..4ff4bbd 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,14 @@ static int isp_probe(struct platform_device *pdev)
 		}
 	}
 
+	isp->syscon = syscon_regmap_lookup_by_pdevname("syscon.0");
+	isp->syscon_offset = isp_res_maps[m].syscon_offset;
+	isp->phy_type = isp_res_maps[m].phy_type;
+	if (IS_ERR(isp->syscon)) {
+		ret = PTR_ERR(isp->syscon);
+		goto error_isp;
+	}
+
 	/* 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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 10/18] omap3isp: Move the syscon register out of the ISP register maps

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

(CC'ing linux-omap and Tony)

On Saturday 07 March 2015 23:41:07 Sakari Ailus wrote:
> 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>
> ---
>  arch/arm/boot/dts/omap3.dtsi                |    2 +-
>  arch/arm/mach-omap2/devices.c               |   10 ----------
>  drivers/media/platform/omap3isp/isp.c       |   19 +++++++++++++++----
>  drivers/media/platform/omap3isp/isp.h       |   19 +++++++++++++++++--
>  drivers/media/platform/omap3isp/ispcsiphy.c |   20 +++++++++-----------

You might be asked to split the patch into two, let's see what Tony says.

>  5 files changed, 42 insertions(+), 28 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
> index 01b7111..fe0b293 100644
> --- a/arch/arm/boot/dts/omap3.dtsi
> +++ b/arch/arm/boot/dts/omap3.dtsi
> @@ -183,7 +183,7 @@
> 
>  		omap3_scm_general: tisyscon@48002270 {
>  			compatible = "syscon";
> -			reg = <0x48002270 0x2f0>;
> +			reg = <0x48002270 0x2f4>;
>  		};
> 
>  		pbias_regulator: pbias_regulator {
> 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..4ff4bbd 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,14 @@ static int isp_probe(struct platform_device *pdev)
>  		}
>  	}
> 
> +	isp->syscon = syscon_regmap_lookup_by_pdevname("syscon.0");
> +	isp->syscon_offset = isp_res_maps[m].syscon_offset;
> +	isp->phy_type = isp_res_maps[m].phy_type;

You could move those two lines after the error check to keep the check closer 
to the source of error.

Apart from that,

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

> +	if (IS_ERR(isp->syscon)) {
> +		ret = PTR_ERR(isp->syscon);
> +		goto error_isp;
> +	}
> +
>  	/* 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);
>  }

-- 
Regards,

Laurent Pinchart

Re: [RFC 10/18] omap3isp: Move the syscon register out of the ISP register maps

[Sakari Ailus]

Hi Laurent,

On Sun, Mar 08, 2015 at 01:34:17AM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> (CC'ing linux-omap and Tony)

Thanks.

> On Saturday 07 March 2015 23:41:07 Sakari Ailus wrote:
> > 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-X3B1VOXEql0@public.gmane.org>
> > ---
> >  arch/arm/boot/dts/omap3.dtsi                |    2 +-
> >  arch/arm/mach-omap2/devices.c               |   10 ----------
> >  drivers/media/platform/omap3isp/isp.c       |   19 +++++++++++++++----
> >  drivers/media/platform/omap3isp/isp.h       |   19 +++++++++++++++++--
> >  drivers/media/platform/omap3isp/ispcsiphy.c |   20 +++++++++-----------
> 
> You might be asked to split the patch into two, let's see what Tony says.
> 
> >  5 files changed, 42 insertions(+), 28 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
> > index 01b7111..fe0b293 100644
> > --- a/arch/arm/boot/dts/omap3.dtsi
> > +++ b/arch/arm/boot/dts/omap3.dtsi
> > @@ -183,7 +183,7 @@
> > 
> >  		omap3_scm_general: tisyscon@48002270 {
> >  			compatible = "syscon";
> > -			reg = <0x48002270 0x2f0>;
> > +			reg = <0x48002270 0x2f4>;
> >  		};
> > 
> >  		pbias_regulator: pbias_regulator {
> > 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..4ff4bbd 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,14 @@ static int isp_probe(struct platform_device *pdev)
> >  		}
> >  	}
> > 
> > +	isp->syscon = syscon_regmap_lookup_by_pdevname("syscon.0");
> > +	isp->syscon_offset = isp_res_maps[m].syscon_offset;
> > +	isp->phy_type = isp_res_maps[m].phy_type;
> 
> You could move those two lines after the error check to keep the check closer 
> to the source of error.

Ack.

> Apart from that,
> 
> Acked-by: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>

Thanks for the acks!

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org	XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 10/18] omap3isp: Move the syscon register out of the ISP register maps

[Tony Lindgren]

* Sakari Ailus <sakari.ailus@iki.fi> [150307 15:44]:
> Hi Laurent,
> 
> On Sun, Mar 08, 2015 at 01:34:17AM +0200, Laurent Pinchart wrote:
> > Hi Sakari,
> > 
> > Thank you for the patch.
> > 
> > (CC'ing linux-omap and Tony)
> 
> Thanks.
> 
> > On Saturday 07 March 2015 23:41:07 Sakari Ailus wrote:
> > > 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>
> > > ---
> > >  arch/arm/boot/dts/omap3.dtsi                |    2 +-
> > >  arch/arm/mach-omap2/devices.c               |   10 ----------
> > >  drivers/media/platform/omap3isp/isp.c       |   19 +++++++++++++++----
> > >  drivers/media/platform/omap3isp/isp.h       |   19 +++++++++++++++++--
> > >  drivers/media/platform/omap3isp/ispcsiphy.c |   20 +++++++++-----------
> > 
> > You might be asked to split the patch into two, let's see what Tony says.
> > 
> > >  5 files changed, 42 insertions(+), 28 deletions(-)
> > > 
> > > diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
> > > index 01b7111..fe0b293 100644
> > > --- a/arch/arm/boot/dts/omap3.dtsi
> > > +++ b/arch/arm/boot/dts/omap3.dtsi
> > > @@ -183,7 +183,7 @@
> > > 
> > >  		omap3_scm_general: tisyscon@48002270 {
> > >  			compatible = "syscon";
> > > -			reg = <0x48002270 0x2f0>;
> > > +			reg = <0x48002270 0x2f4>;
> > >  		};
> > > 
> > >  		pbias_regulator: pbias_regulator {

Can you please send the above dts change separately as a fix describing
what goes wrong? Let's get that out of the way for the -rc, otherwise
we're going to probably get conflicts with Tero's dts changes.

> > > 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,
> > >  	}

Looks good to me, teel free to merge this part along with the other
isp changes:

Acked-by: Tony Lindgren <tony@atomide.com>


> > > diff --git a/drivers/media/platform/omap3isp/isp.c
> > > b/drivers/media/platform/omap3isp/isp.c index 68d7edfc..4ff4bbd 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,14 @@ static int isp_probe(struct platform_device *pdev)
> > >  		}
> > >  	}
> > > 
> > > +	isp->syscon = syscon_regmap_lookup_by_pdevname("syscon.0");
> > > +	isp->syscon_offset = isp_res_maps[m].syscon_offset;
> > > +	isp->phy_type = isp_res_maps[m].phy_type;
> > 
> > You could move those two lines after the error check to keep the check closer 
> > to the source of error.
> 
> Ack.
> 
> > Apart from that,
> > 
> > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Thanks for the acks!
> 
> -- 
> Kind regards,
> 
> Sakari Ailus
> e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

Re: [RFC 10/18] omap3isp: Move the syscon register out of the ISP register maps

[Sakari Ailus]

Hi Tony,

Thanks for the comments!!

On Mon, Mar 09, 2015 at 08:20:38AM -0700, Tony Lindgren wrote:
> * Sakari Ailus <sakari.ailus@iki.fi> [150307 15:44]:
> > Hi Laurent,
> > 
> > On Sun, Mar 08, 2015 at 01:34:17AM +0200, Laurent Pinchart wrote:
> > > Hi Sakari,
> > > 
> > > Thank you for the patch.
> > > 
> > > (CC'ing linux-omap and Tony)
> > 
> > Thanks.
> > 
> > > On Saturday 07 March 2015 23:41:07 Sakari Ailus wrote:
> > > > 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>
> > > > ---
> > > >  arch/arm/boot/dts/omap3.dtsi                |    2 +-
> > > >  arch/arm/mach-omap2/devices.c               |   10 ----------
> > > >  drivers/media/platform/omap3isp/isp.c       |   19 +++++++++++++++----
> > > >  drivers/media/platform/omap3isp/isp.h       |   19 +++++++++++++++++--
> > > >  drivers/media/platform/omap3isp/ispcsiphy.c |   20 +++++++++-----------
> > > 
> > > You might be asked to split the patch into two, let's see what Tony says.
> > > 
> > > >  5 files changed, 42 insertions(+), 28 deletions(-)
> > > > 
> > > > diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
> > > > index 01b7111..fe0b293 100644
> > > > --- a/arch/arm/boot/dts/omap3.dtsi
> > > > +++ b/arch/arm/boot/dts/omap3.dtsi
> > > > @@ -183,7 +183,7 @@
> > > > 
> > > >  		omap3_scm_general: tisyscon@48002270 {
> > > >  			compatible = "syscon";
> > > > -			reg = <0x48002270 0x2f0>;
> > > > +			reg = <0x48002270 0x2f4>;
> > > >  		};
> > > > 
> > > >  		pbias_regulator: pbias_regulator {
> 
> Can you please send the above dts change separately as a fix describing
> what goes wrong? Let's get that out of the way for the -rc, otherwise
> we're going to probably get conflicts with Tero's dts changes.

Sure.

There's one register that didn't used to be mapped to syscon.

> > > > 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,
> > > >  	}
> 
> Looks good to me, teel free to merge this part along with the other
> isp changes:
> 
> Acked-by: Tony Lindgren <tony@atomide.com>

Thanks!

-- 
Regards,

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

Re: [RFC 10/18] omap3isp: Move the syscon register out of the ISP register maps

[Laurent Pinchart]

Hi Sakari,

On Saturday 07 March 2015 23:41:07 Sakari Ailus wrote:
> 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>
> ---
>  arch/arm/boot/dts/omap3.dtsi                |    2 +-
>  arch/arm/mach-omap2/devices.c               |   10 ----------
>  drivers/media/platform/omap3isp/isp.c       |   19 +++++++++++++++----
>  drivers/media/platform/omap3isp/isp.h       |   19 +++++++++++++++++--
>  drivers/media/platform/omap3isp/ispcsiphy.c |   20 +++++++++-----------

I've noticed another issue, you need a "select MFD_SYSCON" in Kconfig.

>  5 files changed, 42 insertions(+), 28 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
> index 01b7111..fe0b293 100644
> --- a/arch/arm/boot/dts/omap3.dtsi
> +++ b/arch/arm/boot/dts/omap3.dtsi
> @@ -183,7 +183,7 @@
> 
>  		omap3_scm_general: tisyscon@48002270 {
>  			compatible = "syscon";
> -			reg = <0x48002270 0x2f0>;
> +			reg = <0x48002270 0x2f4>;
>  		};
> 
>  		pbias_regulator: pbias_regulator {
> 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..4ff4bbd 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,14 @@ static int isp_probe(struct platform_device *pdev)
>  		}
>  	}
> 
> +	isp->syscon = syscon_regmap_lookup_by_pdevname("syscon.0");
> +	isp->syscon_offset = isp_res_maps[m].syscon_offset;
> +	isp->phy_type = isp_res_maps[m].phy_type;
> +	if (IS_ERR(isp->syscon)) {
> +		ret = PTR_ERR(isp->syscon);
> +		goto error_isp;
> +	}
> +
>  	/* 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);
>  }

-- 
Regards,

Laurent Pinchart

Re: [RFC 10/18] omap3isp: Move the syscon register out of the ISP register maps

[Sakari Ailus]

On Mon, Mar 16, 2015 at 02:19:04AM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Saturday 07 March 2015 23:41:07 Sakari Ailus wrote:
> > 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-X3B1VOXEql0@public.gmane.org>
> > ---
> >  arch/arm/boot/dts/omap3.dtsi                |    2 +-
> >  arch/arm/mach-omap2/devices.c               |   10 ----------
> >  drivers/media/platform/omap3isp/isp.c       |   19 +++++++++++++++----
> >  drivers/media/platform/omap3isp/isp.h       |   19 +++++++++++++++++--
> >  drivers/media/platform/omap3isp/ispcsiphy.c |   20 +++++++++-----------
> 
> I've noticed another issue, you need a "select MFD_SYSCON" in Kconfig.

Thanks, fixed!

-- 
Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org	XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC 13/18] 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.

Signed-off-by: Sakari Ailus <sakari.ailus-X3B1VOXEql0@public.gmane.org>
---
 drivers/media/v4l2-core/v4l2-of.c |   21 ++++++++++++++++-----
 include/media/v4l2-of.h           |    3 +++
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-of.c b/drivers/media/v4l2-core/v4l2-of.c
index b4ed9a9..a7a855e 100644
--- a/drivers/media/v4l2-core/v4l2-of.c
+++ b/drivers/media/v4l2-core/v4l2-of.c
@@ -23,7 +23,6 @@ static void 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;
@@ -34,14 +33,26 @@ static void v4l2_of_parse_csi_bus(const struct device_node *node,
 		const __be32 *lane = NULL;
 		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;
+		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 (!of_property_read_u32(node, "clock-lanes", &v)) {
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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 13/18] v4l: of: Read lane-polarity endpoint property

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Saturday 07 March 2015 23:41:10 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.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> ---
>  drivers/media/v4l2-core/v4l2-of.c |   21 ++++++++++++++++-----
>  include/media/v4l2-of.h           |    3 +++
>  2 files changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-of.c
> b/drivers/media/v4l2-core/v4l2-of.c index b4ed9a9..a7a855e 100644
> --- a/drivers/media/v4l2-core/v4l2-of.c
> +++ b/drivers/media/v4l2-core/v4l2-of.c
> @@ -23,7 +23,6 @@ static void 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;
> @@ -34,14 +33,26 @@ static void v4l2_of_parse_csi_bus(const struct
> device_node *node, const __be32 *lane = NULL;
>  		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;
> +		int i;

Could you please use unsigned int instead of int as the loop index can't have 
negative value ? Feel free to fix the index in the previous loop too :-)

> +
> +		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;
> +		}

Should we check that i == num_data_lines + 1 ?

>  	}
> 
>  	if (!of_property_read_u32(node, "clock-lanes", &v)) {
> 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];
>  };
> 
>  /**

-- 
Regards,

Laurent Pinchart

Re: [RFC 13/18] v4l: of: Read lane-polarity endpoint property

[Sakari Ailus]

Hi Laurent,

On Sun, Mar 08, 2015 at 01:49:26AM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Saturday 07 March 2015 23:41:10 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.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> > ---
> >  drivers/media/v4l2-core/v4l2-of.c |   21 ++++++++++++++++-----
> >  include/media/v4l2-of.h           |    3 +++
> >  2 files changed, 19 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-of.c
> > b/drivers/media/v4l2-core/v4l2-of.c index b4ed9a9..a7a855e 100644
> > --- a/drivers/media/v4l2-core/v4l2-of.c
> > +++ b/drivers/media/v4l2-core/v4l2-of.c
> > @@ -23,7 +23,6 @@ static void 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;
> > @@ -34,14 +33,26 @@ static void v4l2_of_parse_csi_bus(const struct
> > device_node *node, const __be32 *lane = NULL;
> >  		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;
> > +		int i;
> 
> Could you please use unsigned int instead of int as the loop index can't have 
> negative value ? Feel free to fix the index in the previous loop too :-)

Fixed both.

> > +
> > +		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;
> > +		}
> 
> Should we check that i == num_data_lines + 1 ?

Good question. I think I'd just replace this with
of_property_read_u32_array() instead, how about that? Then there would have
to be at least as many lane polarities defined as there are lanes (data and
clock). Defining more wouldn't be an error.

-- 
Regards,

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

Re: [RFC 13/18] v4l: of: Read lane-polarity endpoint property

[Sakari Ailus]

On Fri, Mar 13, 2015 at 12:23:27AM +0200, Sakari Ailus wrote:
...
> > > +
> > > +		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;
> > > +		}
> > 
> > Should we check that i == num_data_lines + 1 ?
> 
> Good question. I think I'd just replace this with
> of_property_read_u32_array() instead, how about that? Then there would have
> to be at least as many lane polarities defined as there are lanes (data and
> clock). Defining more wouldn't be an error.

Oh, I missed the variable in the struct is bool. I'll just add the check.

-- 
Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org	XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC 17/18] arm: dts: n950, n9: Add primary camera support

[Sakari Ailus]

Add support for the primary camera of the Nokia N950 and N9.

Signed-off-by: Sakari Ailus <sakari.ailus-X3B1VOXEql0@public.gmane.org>
---
 arch/arm/boot/dts/omap3-n9.dts       |   39 ++++++++++++++++++++++++++++++++++
 arch/arm/boot/dts/omap3-n950-n9.dtsi |    4 ----
 arch/arm/boot/dts/omap3-n950.dts     |   39 ++++++++++++++++++++++++++++++++++
 3 files changed, 78 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/omap3-n9.dts b/arch/arm/boot/dts/omap3-n9.dts
index 9938b5d..05f32ae 100644
--- a/arch/arm/boot/dts/omap3-n9.dts
+++ b/arch/arm/boot/dts/omap3-n9.dts
@@ -16,3 +16,42 @@
 	model = "Nokia N9";
 	compatible = "nokia,omap3-n9", "ti,omap36xx", "ti,omap3";
 };
+
+&i2c2 {
+	clock-frequency = <400000>;
+
+	smia_1: camera@10 {
+		compatible = "nokia,smia";
+		reg = <0x10>;
+		/* No reset gpio */
+		vana-supply = <&vaux3>;
+		clocks = <&omap3_isp 0>;
+		clock-frequency = <9600000>;
+		nokia,nvm-size = <1024>; /* 16 * 64 */
+		link-frequencies = /bits/ 64 <199200000 210000000 499200000>;
+		port {
+			smia_1_1: endpoint {
+				clock-lanes = <0>;
+				data-lanes = <1 2>;
+				remote-endpoint = <&csi2a_ep>;
+			};
+		};
+	};
+};
+
+&omap3_isp {
+	vdd-csiphy1-supply = <&vaux2>;
+	vdd-csiphy2-supply = <&vaux2>;
+	ports {
+		port@2 {
+			reg = <2>;
+			csi2a_ep: endpoint {
+				remote-endpoint = <&smia_1_1>;
+				clock-lanes = <2>;
+				data-lanes = <1 3>;
+				crc = <1>;
+				lane-polarity = <1 1 1>;
+			};
+		};
+	};
+};
diff --git a/arch/arm/boot/dts/omap3-n950-n9.dtsi b/arch/arm/boot/dts/omap3-n950-n9.dtsi
index c41db94..51e5043 100644
--- a/arch/arm/boot/dts/omap3-n950-n9.dtsi
+++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi
@@ -86,10 +86,6 @@
 	regulator-max-microvolt = <2800000>;
 };
 
-&i2c2 {
-	clock-frequency = <400000>;
-};
-
 &i2c3 {
 	clock-frequency = <400000>;
 };
diff --git a/arch/arm/boot/dts/omap3-n950.dts b/arch/arm/boot/dts/omap3-n950.dts
index 261c558..2b2ed9c 100644
--- a/arch/arm/boot/dts/omap3-n950.dts
+++ b/arch/arm/boot/dts/omap3-n950.dts
@@ -16,3 +16,42 @@
 	model = "Nokia N950";
 	compatible = "nokia,omap3-n950", "ti,omap36xx", "ti,omap3";
 };
+
+&i2c2 {
+	clock-frequency = <400000>;
+
+	smia_1: camera@10 {
+		compatible = "nokia,smia";
+		reg = <0x10>;
+		/* No reset gpio */
+		vana-supply = <&vaux3>;
+		clocks = <&omap3_isp 0>;
+		clock-frequency = <9600000>;
+		nokia,nvm-size = <1024>; /* 16 * 64 */
+		link-frequencies = /bits/ 64 <210000000 333600000 398400000>;
+		port {
+			smia_1_1: endpoint {
+				clock-lanes = <0>;
+				data-lanes = <1 2>;
+				remote-endpoint = <&csi2a_ep>;
+			};
+		};
+	};
+};
+
+&omap3_isp {
+	vdd-csiphy1-supply = <&vaux2>;
+	vdd-csiphy2-supply = <&vaux2>;
+	ports {
+		port@2 {
+			reg = <2>;
+			csi2a_ep: endpoint {
+				remote-endpoint = <&smia_1_1>;
+				clock-lanes = <2>;
+				data-lanes = <3 1>;
+				crc = <1>;
+				lane-polarity = <1 1 1>;
+			};
+		};
+	};
+};
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 17/18] arm: dts: n950, n9: Add primary camera support

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Saturday 07 March 2015 23:41:14 Sakari Ailus wrote:
> Add support for the primary camera of the Nokia N950 and N9.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus-X3B1VOXEql0@public.gmane.org>
> ---
>  arch/arm/boot/dts/omap3-n9.dts       |   39 +++++++++++++++++++++++++++++++
>  arch/arm/boot/dts/omap3-n950-n9.dtsi |    4 ----
>  arch/arm/boot/dts/omap3-n950.dts     |   39 +++++++++++++++++++++++++++++++
>  3 files changed, 78 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/omap3-n9.dts b/arch/arm/boot/dts/omap3-n9.dts
> index 9938b5d..05f32ae 100644
> --- a/arch/arm/boot/dts/omap3-n9.dts
> +++ b/arch/arm/boot/dts/omap3-n9.dts
> @@ -16,3 +16,42 @@
>  	model = "Nokia N9";
>  	compatible = "nokia,omap3-n9", "ti,omap36xx", "ti,omap3";
>  };
> +
> +&i2c2 {
> +	clock-frequency = <400000>;
> +
> +	smia_1: camera@10 {
> +		compatible = "nokia,smia";
> +		reg = <0x10>;
> +		/* No reset gpio */
> +		vana-supply = <&vaux3>;
> +		clocks = <&omap3_isp 0>;
> +		clock-frequency = <9600000>;
> +		nokia,nvm-size = <1024>; /* 16 * 64 */

You could actually specify that as "<(16 * 64)>".

> +		link-frequencies = /bits/ 64 <199200000 210000000 499200000>;
> +		port {
> +			smia_1_1: endpoint {
> +				clock-lanes = <0>;
> +				data-lanes = <1 2>;
> +				remote-endpoint = <&csi2a_ep>;
> +			};
> +		};
> +	};
> +};
> +
> +&omap3_isp {
> +	vdd-csiphy1-supply = <&vaux2>;
> +	vdd-csiphy2-supply = <&vaux2>;
> +	ports {
> +		port@2 {
> +			reg = <2>;
> +			csi2a_ep: endpoint {
> +				remote-endpoint = <&smia_1_1>;
> +				clock-lanes = <2>;
> +				data-lanes = <1 3>;
> +				crc = <1>;
> +				lane-polarity = <1 1 1>;
> +			};
> +		};
> +	};
> +};
> diff --git a/arch/arm/boot/dts/omap3-n950-n9.dtsi
> b/arch/arm/boot/dts/omap3-n950-n9.dtsi index c41db94..51e5043 100644
> --- a/arch/arm/boot/dts/omap3-n950-n9.dtsi
> +++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi
> @@ -86,10 +86,6 @@
>  	regulator-max-microvolt = <2800000>;
>  };
> 
> -&i2c2 {
> -	clock-frequency = <400000>;
> -};
> -

What's the reason for moving this to the N9 and N950 DT files as you keep the 
same value in both ?

>  &i2c3 {
>  	clock-frequency = <400000>;
>  };
> diff --git a/arch/arm/boot/dts/omap3-n950.dts
> b/arch/arm/boot/dts/omap3-n950.dts index 261c558..2b2ed9c 100644
> --- a/arch/arm/boot/dts/omap3-n950.dts
> +++ b/arch/arm/boot/dts/omap3-n950.dts
> @@ -16,3 +16,42 @@
>  	model = "Nokia N950";
>  	compatible = "nokia,omap3-n950", "ti,omap36xx", "ti,omap3";
>  };
> +
> +&i2c2 {
> +	clock-frequency = <400000>;
> +
> +	smia_1: camera@10 {
> +		compatible = "nokia,smia";
> +		reg = <0x10>;
> +		/* No reset gpio */
> +		vana-supply = <&vaux3>;
> +		clocks = <&omap3_isp 0>;
> +		clock-frequency = <9600000>;
> +		nokia,nvm-size = <1024>; /* 16 * 64 */
> +		link-frequencies = /bits/ 64 <210000000 333600000 398400000>;
> +		port {
> +			smia_1_1: endpoint {
> +				clock-lanes = <0>;
> +				data-lanes = <1 2>;
> +				remote-endpoint = <&csi2a_ep>;
> +			};
> +		};
> +	};
> +};
> +
> +&omap3_isp {
> +	vdd-csiphy1-supply = <&vaux2>;
> +	vdd-csiphy2-supply = <&vaux2>;
> +	ports {
> +		port@2 {
> +			reg = <2>;
> +			csi2a_ep: endpoint {
> +				remote-endpoint = <&smia_1_1>;
> +				clock-lanes = <2>;
> +				data-lanes = <3 1>;
> +				crc = <1>;
> +				lane-polarity = <1 1 1>;
> +			};
> +		};
> +	};
> +};

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 17/18] arm: dts: n950, n9: Add primary camera support

[Sakari Ailus]

Hi Laurent,

On Sun, Mar 08, 2015 at 01:56:13AM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Saturday 07 March 2015 23:41:14 Sakari Ailus wrote:
> > Add support for the primary camera of the Nokia N950 and N9.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus-X3B1VOXEql0@public.gmane.org>
> > ---
> >  arch/arm/boot/dts/omap3-n9.dts       |   39 +++++++++++++++++++++++++++++++
> >  arch/arm/boot/dts/omap3-n950-n9.dtsi |    4 ----
> >  arch/arm/boot/dts/omap3-n950.dts     |   39 +++++++++++++++++++++++++++++++
> >  3 files changed, 78 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/omap3-n9.dts b/arch/arm/boot/dts/omap3-n9.dts
> > index 9938b5d..05f32ae 100644
> > --- a/arch/arm/boot/dts/omap3-n9.dts
> > +++ b/arch/arm/boot/dts/omap3-n9.dts
> > @@ -16,3 +16,42 @@
> >  	model = "Nokia N9";
> >  	compatible = "nokia,omap3-n9", "ti,omap36xx", "ti,omap3";
> >  };
> > +
> > +&i2c2 {
> > +	clock-frequency = <400000>;
> > +
> > +	smia_1: camera@10 {
> > +		compatible = "nokia,smia";
> > +		reg = <0x10>;
> > +		/* No reset gpio */
> > +		vana-supply = <&vaux3>;
> > +		clocks = <&omap3_isp 0>;
> > +		clock-frequency = <9600000>;
> > +		nokia,nvm-size = <1024>; /* 16 * 64 */
> 
> You could actually specify that as "<(16 * 64)>".

Will fix.

> > +		link-frequencies = /bits/ 64 <199200000 210000000 499200000>;
> > +		port {
> > +			smia_1_1: endpoint {
> > +				clock-lanes = <0>;
> > +				data-lanes = <1 2>;
> > +				remote-endpoint = <&csi2a_ep>;
> > +			};
> > +		};
> > +	};
> > +};
> > +
> > +&omap3_isp {
> > +	vdd-csiphy1-supply = <&vaux2>;
> > +	vdd-csiphy2-supply = <&vaux2>;
> > +	ports {
> > +		port@2 {
> > +			reg = <2>;
> > +			csi2a_ep: endpoint {
> > +				remote-endpoint = <&smia_1_1>;
> > +				clock-lanes = <2>;
> > +				data-lanes = <1 3>;
> > +				crc = <1>;
> > +				lane-polarity = <1 1 1>;
> > +			};
> > +		};
> > +	};
> > +};
> > diff --git a/arch/arm/boot/dts/omap3-n950-n9.dtsi
> > b/arch/arm/boot/dts/omap3-n950-n9.dtsi index c41db94..51e5043 100644
> > --- a/arch/arm/boot/dts/omap3-n950-n9.dtsi
> > +++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi
> > @@ -86,10 +86,6 @@
> >  	regulator-max-microvolt = <2800000>;
> >  };
> > 
> > -&i2c2 {
> > -	clock-frequency = <400000>;
> > -};
> > -
> 
> What's the reason for moving this to the N9 and N950 DT files as you keep the 
> same value in both ?

Just before submitting the patches I thought someone might ask. :-D

I'll fix that.

> >  &i2c3 {
> >  	clock-frequency = <400000>;
> >  };
> > diff --git a/arch/arm/boot/dts/omap3-n950.dts
> > b/arch/arm/boot/dts/omap3-n950.dts index 261c558..2b2ed9c 100644
> > --- a/arch/arm/boot/dts/omap3-n950.dts
> > +++ b/arch/arm/boot/dts/omap3-n950.dts
> > @@ -16,3 +16,42 @@
> >  	model = "Nokia N950";
> >  	compatible = "nokia,omap3-n950", "ti,omap36xx", "ti,omap3";
> >  };
> > +
> > +&i2c2 {
> > +	clock-frequency = <400000>;
> > +
> > +	smia_1: camera@10 {
> > +		compatible = "nokia,smia";
> > +		reg = <0x10>;
> > +		/* No reset gpio */
> > +		vana-supply = <&vaux3>;
> > +		clocks = <&omap3_isp 0>;
> > +		clock-frequency = <9600000>;
> > +		nokia,nvm-size = <1024>; /* 16 * 64 */
> > +		link-frequencies = /bits/ 64 <210000000 333600000 398400000>;
> > +		port {
> > +			smia_1_1: endpoint {
> > +				clock-lanes = <0>;
> > +				data-lanes = <1 2>;
> > +				remote-endpoint = <&csi2a_ep>;
> > +			};
> > +		};
> > +	};
> > +};
> > +
> > +&omap3_isp {
> > +	vdd-csiphy1-supply = <&vaux2>;
> > +	vdd-csiphy2-supply = <&vaux2>;
> > +	ports {
> > +		port@2 {
> > +			reg = <2>;
> > +			csi2a_ep: endpoint {
> > +				remote-endpoint = <&smia_1_1>;
> > +				clock-lanes = <2>;
> > +				data-lanes = <3 1>;
> > +				crc = <1>;
> > +				lane-polarity = <1 1 1>;
> > +			};
> > +		};
> > +	};
> > +};
> 

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org	XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC 18/18] 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-X3B1VOXEql0@public.gmane.org>
---
 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 2c9bc0d..e4a78bb 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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 18/18] omap3isp: Deprecate platform data support

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Saturday 07 March 2015 23:41:15 Sakari Ailus wrote:
> 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>

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 2c9bc0d..e4a78bb 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

Re: [RFC 18/18] omap3isp: Deprecate platform data support

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 339 bytes --]

Hi,

[+CC Tony]

On Sat, Mar 07, 2015 at 11:41:15PM +0200, Sakari Ailus wrote:
> Print a warning when the driver is used with platform data. Existing
> platform data user should move to DT now.

I guess this should become a more visible warning on OMAP SoC level,
since platform data based boot will be deprecated completly.

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFC 18/18] omap3isp: Deprecate platform data support

[Tony Lindgren]

* Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> [150313 02:41]:
> Hi,
> 
> [+CC Tony]
> 
> On Sat, Mar 07, 2015 at 11:41:15PM +0200, Sakari Ailus wrote:
> > Print a warning when the driver is used with platform data. Existing
> > platform data user should move to DT now.
> 
> I guess this should become a more visible warning on OMAP SoC level,
> since platform data based boot will be deprecated completly.

Yeah. We should need platform data in addition to device tree data in
only very few cases. Basically only when there's some arch specific
callback function that needs to be passed to the driver.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC 07/18] 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>
---
 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

Re: [RFC 07/18] omap3isp: Rename regulators to better suit the Device Tree

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Saturday 07 March 2015 23:41:04 Sakari Ailus wrote:
> 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
>  	 *

-- 
Regards,

Laurent Pinchart

[RFC 08/18] 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>
---
 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

Re: [RFC 08/18] omap3isp: Calculate vpclk_div for CSI-2

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Saturday 07 March 2015 23:41:05 Sakari Ailus wrote:
> 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;
>  };

-- 
Regards,

Laurent Pinchart

[RFC 11/18] 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>
---
 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 4ff4bbd..7804895 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");
 	isp->syscon_offset = isp_res_maps[m].syscon_offset;
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

Re: [RFC 11/18] omap3isp: Replace many MMIO regions by two

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

(CC'ing linux-omap and Tony)

On Saturday 07 March 2015 23:41:08 Sakari Ailus wrote:
> 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>

> ---
>  arch/arm/mach-omap2/devices.c         |   66 +------------------
>  arch/arm/mach-omap2/omap34xx.h        |   36 +----------

Once again you might be asked to split this. However, it would be pretty 
painful, so it would be nice if we could merge everything through the Linux 
media tree. You will need an ack from Tony.

>  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 4ff4bbd..7804895 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");
>  	isp->syscon_offset = isp_res_maps[m].syscon_offset;
> 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;
>  };

-- 
Regards,

Laurent Pinchart

Re: [RFC 11/18] omap3isp: Replace many MMIO regions by two

[Tony Lindgren]

* Laurent Pinchart <laurent.pinchart@ideasonboard.com> [150307 15:43]:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> (CC'ing linux-omap and Tony)
> 
> On Saturday 07 March 2015 23:41:08 Sakari Ailus wrote:
> > 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>
> 
> > ---
> >  arch/arm/mach-omap2/devices.c         |   66 +------------------
> >  arch/arm/mach-omap2/omap34xx.h        |   36 +----------
> 
> Once again you might be asked to split this. However, it would be pretty 
> painful, so it would be nice if we could merge everything through the Linux 
> media tree. You will need an ack from Tony.

These changes look fine to me and should not conflict with anything
I'm planning to queue, so please feel free to take them along with
the other isp changes:

Acked-by: Tony Lindgren <tony@atomide.com>
 
> >  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 4ff4bbd..7804895 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");
> >  	isp->syscon_offset = isp_res_maps[m].syscon_offset;
> > 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;
> >  };
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 

[RFC 12/18] 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 |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt b/Documentation/devicetree/bindings/media/video-interfaces.txt
index 571b4c6..058d1e6 100644
--- a/Documentation/devicetree/bindings/media/video-interfaces.txt
+++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
@@ -106,6 +106,11 @@ 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.
+  This property is valid for serial busses only.
 
 
 Example
-- 
1.7.10.4

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

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

(CC'ing Sylwester)

On Saturday 07 March 2015 23:41:09 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>
> ---
>  Documentation/devicetree/bindings/media/video-interfaces.txt |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt
> b/Documentation/devicetree/bindings/media/video-interfaces.txt index
> 571b4c6..058d1e6 100644
> --- a/Documentation/devicetree/bindings/media/video-interfaces.txt
> +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
> @@ -106,6 +106,11 @@ 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).

Would it make sense to add #define's for this ?

> The length of the array
> +  should be the combined length of data-lanes and clock-lanes
> properties.
> +  This property is valid for serial busses only.

You should also document what happens when the property is omitted.

>  Example

-- 
Regards,

Laurent Pinchart

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

[Sakari Ailus]

Hi Laurent,

On Sun, Mar 08, 2015 at 01:46:02AM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> (CC'ing Sylwester)
> 
> On Saturday 07 March 2015 23:41:09 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>
> > ---
> >  Documentation/devicetree/bindings/media/video-interfaces.txt |    5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt
> > b/Documentation/devicetree/bindings/media/video-interfaces.txt index
> > 571b4c6..058d1e6 100644
> > --- a/Documentation/devicetree/bindings/media/video-interfaces.txt
> > +++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
> > @@ -106,6 +106,11 @@ 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).
> 
> Would it make sense to add #define's for this ?

Good question. I don't really have too much of an opinion. I think I'd just
use a number until someone else needs this. :-)

> > The length of the array
> > +  should be the combined length of data-lanes and clock-lanes
> > properties.
> > +  This property is valid for serial busses only.
> 
> You should also document what happens when the property is omitted.

Will add.

-- 
Regards,

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

[RFC 14/18] dt: bindings: Add bindings for omap3isp

[Sakari Ailus]

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
---
 .../devicetree/bindings/media/ti,omap3isp.txt      |   64 ++++++++++++++++++++
 MAINTAINERS                                        |    1 +
 2 files changed, 65 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/ti,omap3isp.txt

diff --git a/Documentation/devicetree/bindings/media/ti,omap3isp.txt b/Documentation/devicetree/bindings/media/ti,omap3isp.txt
new file mode 100644
index 0000000..2059524
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/ti,omap3isp.txt
@@ -0,0 +1,64 @@
+OMAP 3 ISP Device Tree bindings
+===============================
+
+More documentation on these bindings is available in
+video-interfaces.txt in the same directory.
+
+Required properties
+===================
+
+compatible	: "ti,omap3-isp"
+reg		: a set of two register block physical addresses and
+		  lengths
+interrupts	: the interrupt number
+iommus		: phandle of the IOMMU
+syscon		: syscon phandle and register offset
+ti,phy-type	: 0 -- 3430; 1 -- 3630
+#clock-cells	: Must be 1 --- the ISP provides two external clocks,
+		  cam_xclka and cam_xclkb, at indices 0 and 1,
+		  respectively. Please find more information on common
+		  clock bindings in ../clock/clock-bindings.txt.
+
+Port nodes (optional)
+---------------------
+
+reg		: The interface:
+		  0 - parallel (CCDC)
+		  1 - CSIPHY1 -- CSI2C / CCP2B on 3630;
+		      CSI1 -- CSIb on 3430
+		  2 - CSIPHY2 -- CSI2A / CCP2B on 3630;
+		      CSI2 -- CSIa on 3430
+
+Optional properties
+===================
+
+vdd-csiphy1-supply : voltage supply of the CSI-2 PHY 1
+vdd-csiphy2-supply : voltage supply of the CSI-2 PHY 2
+
+Endpoint nodes
+--------------
+
+lane-polarity	: lane polarity (required on CSI-2)
+		  0 -- not inverted; 1 -- inverted
+data-lanes	: an array of data lanes from 1 to 3. The length can
+		  be either 1 or 2. (required CSI-2)
+clock-lanes	: the clock lane (from 1 to 3). (required on CSI-2)
+
+
+Example
+=======
+
+		omap3_isp: omap3_isp@480bc000 {
+			compatible = "ti,omap3-isp";
+			reg = <0x480bc000 0x12fc
+			       0x480bd800 0x0600>;
+			interrupts = <24>;
+			iommus = <&mmu_isp>;
+			syscon = <&omap3_scm_general 0x2f0>;
+			ti,phy-type = <1>;
+			#clock-cells = <1>;
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+			};
+		};
diff --git a/MAINTAINERS b/MAINTAINERS
index ddc5a8c..cdeef39 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7079,6 +7079,7 @@ L:	linux-media@vger.kernel.org
 S:	Maintained
 F:	drivers/media/platform/omap3isp/
 F:	drivers/staging/media/omap4iss/
+F:	Documentation/devicetree/bindings/media/ti,omap3isp.txt
 
 OMAP USB SUPPORT
 M:	Felipe Balbi <balbi@ti.com>
-- 
1.7.10.4

Re: [RFC 14/18] dt: bindings: Add bindings for omap3isp

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Saturday 07 March 2015 23:41:11 Sakari Ailus wrote:
> Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> ---
>  .../devicetree/bindings/media/ti,omap3isp.txt      |   64 +++++++++++++++++
>  MAINTAINERS                                        |    1 +
>  2 files changed, 65 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/ti,omap3isp.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/ti,omap3isp.txt
> b/Documentation/devicetree/bindings/media/ti,omap3isp.txt new file mode
> 100644
> index 0000000..2059524
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/ti,omap3isp.txt
> @@ -0,0 +1,64 @@
> +OMAP 3 ISP Device Tree bindings
> +===============================
> +
> +More documentation on these bindings is available in
> +video-interfaces.txt in the same directory.
> +
> +Required properties
> +===================
> +
> +compatible	: "ti,omap3-isp"

I would rephrase that using the usual wording as "compatible: Must contain 
"ti,omap3-isp".

> +reg		: a set of two register block physical addresses and
> +		  lengths

We should describe what each set represents and contains.

> +interrupts	: the interrupt number

I would keep the wording generic and refer to interrupt specifier instead of 
interrupt number.

"interrupts: the ISP interrupt specifier"

> +iommus		: phandle of the IOMMU

Similarly,

"iommus: phandle and IOMMU specifier for the IOMMU that serves the ISP"

> +syscon		: syscon phandle and register offset

We should document what the register offset is.

> +ti,phy-type	: 0 -- 3430; 1 -- 3630

Would it make sense to add #define's for this ?

It could also make sense to document/name them "Complex I/O" and "CSIPHY" to 
avoid referring to the SoC that implements them, as the ISP is also found in 
SoCs other than 3430 and 3630.

Could the PHY type be derived from the ES revision that we query at runtime ?

We should also take into account the fact that the DM3730 has officially no 
CSIPHY, but still seems to implement them in practice.

> +#clock-cells	: Must be 1 --- the ISP provides two external clocks,
> +		  cam_xclka and cam_xclkb, at indices 0 and 1,
> +		  respectively. Please find more information on common
> +		  clock bindings in ../clock/clock-bindings.txt.
> +
> +Port nodes (optional)
> +---------------------

This should refer to Documentation/devicetree/bindings/media/video-
interfaces.txt.

> +reg		: The interface:
> +		  0 - parallel (CCDC)
> +		  1 - CSIPHY1 -- CSI2C / CCP2B on 3630;
> +		      CSI1 -- CSIb on 3430
> +		  2 - CSIPHY2 -- CSI2A / CCP2B on 3630;
> +		      CSI2 -- CSIa on 3430
> +
> +Optional properties
> +===================
> +
> +vdd-csiphy1-supply : voltage supply of the CSI-2 PHY 1
> +vdd-csiphy2-supply : voltage supply of the CSI-2 PHY 2
> +
> +Endpoint nodes
> +--------------
> +
> +lane-polarity	: lane polarity (required on CSI-2)
> +		  0 -- not inverted; 1 -- inverted
> +data-lanes	: an array of data lanes from 1 to 3. The length can
> +		  be either 1 or 2. (required CSI-2)

s/required/required on/ ?

> +clock-lanes	: the clock lane (from 1 to 3). (required on CSI-2)
> +
> +
> +Example
> +=======
> +
> +		omap3_isp: omap3_isp@480bc000 {

DT node names traditionally use - as a separator. Furthermore the phandle 
isn't needed. This should thus probably be

	omap3-isp@480bc000 {

> +			compatible = "ti,omap3-isp";
> +			reg = <0x480bc000 0x12fc
> +			       0x480bd800 0x0600>;
> +			interrupts = <24>;
> +			iommus = <&mmu_isp>;
> +			syscon = <&omap3_scm_general 0x2f0>;
> +			ti,phy-type = <1>;
> +			#clock-cells = <1>;
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +			};
> +		};
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ddc5a8c..cdeef39 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7079,6 +7079,7 @@ L:	linux-media@vger.kernel.org
>  S:	Maintained
>  F:	drivers/media/platform/omap3isp/
>  F:	drivers/staging/media/omap4iss/
> +F:	Documentation/devicetree/bindings/media/ti,omap3isp.txt

I would move this line before the other F: entries to keep them alphabetically 
sorted.

>  OMAP USB SUPPORT
>  M:	Felipe Balbi <balbi@ti.com>

-- 
Regards,

Laurent Pinchart

Re: [RFC 14/18] dt: bindings: Add bindings for omap3isp

[Sakari Ailus]

Hi Laurent,

On Thu, Mar 12, 2015 at 01:39:07AM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Saturday 07 March 2015 23:41:11 Sakari Ailus wrote:
> > Signed-off-by: Sakari Ailus <sakari.ailus-X3B1VOXEql0@public.gmane.org>
> > ---
> >  .../devicetree/bindings/media/ti,omap3isp.txt      |   64 +++++++++++++++++
> >  MAINTAINERS                                        |    1 +
> >  2 files changed, 65 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/ti,omap3isp.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/media/ti,omap3isp.txt
> > b/Documentation/devicetree/bindings/media/ti,omap3isp.txt new file mode
> > 100644
> > index 0000000..2059524
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/ti,omap3isp.txt
> > @@ -0,0 +1,64 @@
> > +OMAP 3 ISP Device Tree bindings
> > +===============================
> > +
> > +More documentation on these bindings is available in
> > +video-interfaces.txt in the same directory.
> > +
> > +Required properties
> > +===================
> > +
> > +compatible	: "ti,omap3-isp"
> 
> I would rephrase that using the usual wording as "compatible: Must contain 
> "ti,omap3-isp".

Fixed.

> > +reg		: a set of two register block physical addresses and
> > +		  lengths
> 
> We should describe what each set represents and contains.

As discussed:

reg             : the two registers sets (physical address and length) for the
                  ISP. The first set contains the core ISP registers up to
                  the end of the SBL block. The second set contains the
                  CSI PHYs and receivers registers.

> > +interrupts	: the interrupt number
> 
> I would keep the wording generic and refer to interrupt specifier instead of 
> interrupt number.
> 
> "interrupts: the ISP interrupt specifier"

Fixed.

> > +iommus		: phandle of the IOMMU
> 
> Similarly,
> 
> "iommus: phandle and IOMMU specifier for the IOMMU that serves the ISP"

Ditto.

> > +syscon		: syscon phandle and register offset
> 
> We should document what the register offset is.

This is SoC specific as is the base address. I'm not sure that would be
relevant here. If you think so, shouldn't we also add the device base
addresses and register block lengths?

> > +ti,phy-type	: 0 -- 3430; 1 -- 3630
> 
> Would it make sense to add #define's for this ?

I'll use OMAP3ISP_PHY_TYPE_COMPLEX_IO and OMAP3ISP_PHY_TYPE_CSIPHY as
discussed.

> It could also make sense to document/name them "Complex I/O" and "CSIPHY" to 
> avoid referring to the SoC that implements them, as the ISP is also found in 
> SoCs other than 3430 and 3630.
> 
> Could the PHY type be derived from the ES revision that we query at runtime ?

I think this would work on 3430 and 3630 but I'm not certain about others.

> We should also take into account the fact that the DM3730 has officially no 
> CSIPHY, but still seems to implement them in practice.

The DT sources are for 36xx, but I'd guess it works on 37xx as well, doesn't
it?

> > +#clock-cells	: Must be 1 --- the ISP provides two external clocks,
> > +		  cam_xclka and cam_xclkb, at indices 0 and 1,
> > +		  respectively. Please find more information on common
> > +		  clock bindings in ../clock/clock-bindings.txt.
> > +
> > +Port nodes (optional)
> > +---------------------
> 
> This should refer to Documentation/devicetree/bindings/media/video-
> interfaces.txt.

There's a reference to video-interfaces.txt in the beginning of the file. Do
you think that'd be enough?

> > +reg		: The interface:
> > +		  0 - parallel (CCDC)
> > +		  1 - CSIPHY1 -- CSI2C / CCP2B on 3630;
> > +		      CSI1 -- CSIb on 3430
> > +		  2 - CSIPHY2 -- CSI2A / CCP2B on 3630;
> > +		      CSI2 -- CSIa on 3430
> > +
> > +Optional properties
> > +===================
> > +
> > +vdd-csiphy1-supply : voltage supply of the CSI-2 PHY 1
> > +vdd-csiphy2-supply : voltage supply of the CSI-2 PHY 2
> > +
> > +Endpoint nodes
> > +--------------
> > +
> > +lane-polarity	: lane polarity (required on CSI-2)
> > +		  0 -- not inverted; 1 -- inverted
> > +data-lanes	: an array of data lanes from 1 to 3. The length can
> > +		  be either 1 or 2. (required CSI-2)
> 
> s/required/required on/ ?

Fixed.

> > +clock-lanes	: the clock lane (from 1 to 3). (required on CSI-2)
> > +
> > +
> > +Example
> > +=======
> > +
> > +		omap3_isp: omap3_isp@480bc000 {
> 
> DT node names traditionally use - as a separator. Furthermore the phandle 
> isn't needed. This should thus probably be
> 
> 	omap3-isp@480bc000 {

Fixed.

> > +			compatible = "ti,omap3-isp";
> > +			reg = <0x480bc000 0x12fc
> > +			       0x480bd800 0x0600>;
> > +			interrupts = <24>;
> > +			iommus = <&mmu_isp>;
> > +			syscon = <&omap3_scm_general 0x2f0>;
> > +			ti,phy-type = <1>;
> > +			#clock-cells = <1>;
> > +			ports {
> > +				#address-cells = <1>;
> > +				#size-cells = <0>;
> > +			};
> > +		};
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index ddc5a8c..cdeef39 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -7079,6 +7079,7 @@ L:	linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >  S:	Maintained
> >  F:	drivers/media/platform/omap3isp/
> >  F:	drivers/staging/media/omap4iss/
> > +F:	Documentation/devicetree/bindings/media/ti,omap3isp.txt
> 
> I would move this line before the other F: entries to keep them alphabetically 
> sorted.

Fixed.

> >  OMAP USB SUPPORT
> >  M:	Felipe Balbi <balbi-l0cyMroinI0@public.gmane.org>
> 

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org	XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 14/18] dt: bindings: Add bindings for omap3isp

[Laurent Pinchart]

Hi Sakari,

On Friday 13 March 2015 01:03:21 Sakari Ailus wrote:
> On Thu, Mar 12, 2015 at 01:39:07AM +0200, Laurent Pinchart wrote:
> > On Saturday 07 March 2015 23:41:11 Sakari Ailus wrote:
> > > Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> > > ---
> > > 
> > >  .../devicetree/bindings/media/ti,omap3isp.txt      |   64 +++++++++++++
> > >  MAINTAINERS                                        |    1 +
> > >  2 files changed, 65 insertions(+)
> > >  create mode 100644
> > >  Documentation/devicetree/bindings/media/ti,omap3isp.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/media/ti,omap3isp.txt
> > > b/Documentation/devicetree/bindings/media/ti,omap3isp.txt new file mode
> > > 100644
> > > index 0000000..2059524
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/ti,omap3isp.txt

[snip]

> > > +syscon		: syscon phandle and register offset
> > 
> > We should document what the register offset is.
> 
> This is SoC specific as is the base address. I'm not sure that would be
> relevant here. If you think so, shouldn't we also add the device base
> addresses and register block lengths?

I meant something like

syscon:	the phandle and register offset to the Complex I/O or CSI-PHY 
register.

> > > +ti,phy-type	: 0 -- 3430; 1 -- 3630
> > 
> > Would it make sense to add #define's for this ?
> 
> I'll use OMAP3ISP_PHY_TYPE_COMPLEX_IO and OMAP3ISP_PHY_TYPE_CSIPHY as
> discussed.
> 
> > It could also make sense to document/name them "Complex I/O" and "CSIPHY"
> > to avoid referring to the SoC that implements them, as the ISP is also
> > found in SoCs other than 3430 and 3630.
> > 
> > Could the PHY type be derived from the ES revision that we query at
> > runtime ?
>
> I think this would work on 3430 and 3630 but I'm not certain about others.
> 
> > We should also take into account the fact that the DM3730 has officially
> > no CSIPHY, but still seems to implement them in practice.
> 
> The DT sources are for 36xx, but I'd guess it works on 37xx as well, doesn't
> it?

I think so.

> > > +#clock-cells	: Must be 1 --- the ISP provides two external clocks,
> > > +		  cam_xclka and cam_xclkb, at indices 0 and 1,
> > > +		  respectively. Please find more information on common
> > > +		  clock bindings in ../clock/clock-bindings.txt.
> > > +
> > > +Port nodes (optional)
> > > +---------------------
> > 
> > This should refer to Documentation/devicetree/bindings/media/video-
> > interfaces.txt.
> 
> There's a reference to video-interfaces.txt in the beginning of the file. Do
> you think that'd be enough?

I've missed that. I think you could move the reference here.

> > > +reg		: The interface:
> > > +		  0 - parallel (CCDC)
> > > +		  1 - CSIPHY1 -- CSI2C / CCP2B on 3630;
> > > +		      CSI1 -- CSIb on 3430
> > > +		  2 - CSIPHY2 -- CSI2A / CCP2B on 3630;
> > > +		      CSI2 -- CSIa on 3430

-- 
Regards,

Laurent Pinchart

Re: [RFC 14/18] dt: bindings: Add bindings for omap3isp

[Sakari Ailus]

Hi Laurent,

On Fri, Mar 13, 2015 at 01:11:03AM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Friday 13 March 2015 01:03:21 Sakari Ailus wrote:
> > On Thu, Mar 12, 2015 at 01:39:07AM +0200, Laurent Pinchart wrote:
> > > On Saturday 07 March 2015 23:41:11 Sakari Ailus wrote:
> > > > Signed-off-by: Sakari Ailus <sakari.ailus-X3B1VOXEql0@public.gmane.org>
> > > > ---
> > > > 
> > > >  .../devicetree/bindings/media/ti,omap3isp.txt      |   64 +++++++++++++
> > > >  MAINTAINERS                                        |    1 +
> > > >  2 files changed, 65 insertions(+)
> > > >  create mode 100644
> > > >  Documentation/devicetree/bindings/media/ti,omap3isp.txt
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/media/ti,omap3isp.txt
> > > > b/Documentation/devicetree/bindings/media/ti,omap3isp.txt new file mode
> > > > 100644
> > > > index 0000000..2059524
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/media/ti,omap3isp.txt
> 
> [snip]
> 
> > > > +syscon		: syscon phandle and register offset
> > > 
> > > We should document what the register offset is.
> > 
> > This is SoC specific as is the base address. I'm not sure that would be
> > relevant here. If you think so, shouldn't we also add the device base
> > addresses and register block lengths?
> 
> I meant something like
> 
> syscon:	the phandle and register offset to the Complex I/O or CSI-PHY 
> register.

Oh, I misunderstood you. I'll use that text.

> > > > +ti,phy-type	: 0 -- 3430; 1 -- 3630
> > > 
> > > Would it make sense to add #define's for this ?
> > 
> > I'll use OMAP3ISP_PHY_TYPE_COMPLEX_IO and OMAP3ISP_PHY_TYPE_CSIPHY as
> > discussed.
> > 
> > > It could also make sense to document/name them "Complex I/O" and "CSIPHY"
> > > to avoid referring to the SoC that implements them, as the ISP is also
> > > found in SoCs other than 3430 and 3630.
> > > 
> > > Could the PHY type be derived from the ES revision that we query at
> > > runtime ?
> >
> > I think this would work on 3430 and 3630 but I'm not certain about others.
> > 
> > > We should also take into account the fact that the DM3730 has officially
> > > no CSIPHY, but still seems to implement them in practice.
> > 
> > The DT sources are for 36xx, but I'd guess it works on 37xx as well, doesn't
> > it?
> 
> I think so.
> 
> > > > +#clock-cells	: Must be 1 --- the ISP provides two external clocks,
> > > > +		  cam_xclka and cam_xclkb, at indices 0 and 1,
> > > > +		  respectively. Please find more information on common
> > > > +		  clock bindings in ../clock/clock-bindings.txt.
> > > > +
> > > > +Port nodes (optional)
> > > > +---------------------
> > > 
> > > This should refer to Documentation/devicetree/bindings/media/video-
> > > interfaces.txt.
> > 
> > There's a reference to video-interfaces.txt in the beginning of the file. Do
> > you think that'd be enough?
> 
> I've missed that. I think you could move the reference here.

Done.

-- 
Sakari Ailus
e-mail: sakari.ailus-X3B1VOXEql0@public.gmane.org	XMPP: sailus-PCDdDYkjdNMDXYZnReoRVg@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 14/18] dt: bindings: Add bindings for omap3isp

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 2017 bytes --]

Hi,

On Fri, Mar 13, 2015 at 01:03:21AM +0200, Sakari Ailus wrote:
> [...]
>
> > > +Required properties
> > > +===================
> > > +
> > > +compatible	: "ti,omap3-isp"
> > 
> > I would rephrase that using the usual wording as "compatible: Must contain 
> > "ti,omap3-isp".
>
> [...]
>
> > > +ti,phy-type	: 0 -- 3430; 1 -- 3630
> > 
> > Would it make sense to add #define's for this ?
> 
> I'll use OMAP3ISP_PHY_TYPE_COMPLEX_IO and OMAP3ISP_PHY_TYPE_CSIPHY as
> discussed.
> 
> > It could also make sense to document/name them "Complex I/O" and "CSIPHY" to 
> > avoid referring to the SoC that implements them, as the ISP is also found in 
> > SoCs other than 3430 and 3630.
> > 
> > Could the PHY type be derived from the ES revision that we query at runtime ?
> 
> I think this would work on 3430 and 3630 but I'm not certain about others.
> 
> > We should also take into account the fact that the DM3730 has officially no 
> > CSIPHY, but still seems to implement them in practice.
> 
> The DT sources are for 36xx, but I'd guess it works on 37xx as well, doesn't
> it?

In other drivers this kind of information is often extracted from the
compatible string. For example:

{ .compatible = "ti,omap34xx-isp", .data = OMAP3ISP_PHY_TYPE_COMPLEX_IO, },
{ .compatible = "ti,omap36xx-isp", .data = OMAP3ISP_PHY_TYPE_CSIPHY, },
...

> [...]
>
> > > +Example
> > > +=======
> > > +
> > > +		omap3_isp: omap3_isp@480bc000 {
> > 
> > DT node names traditionally use - as a separator. Furthermore the phandle 
> > isn't needed. This should thus probably be
> > 
> > 	omap3-isp@480bc000 {
> 
> Fixed.

According to ePAPR this should be a generic name (page 19); For
example the i2c node name should be "i2c@address" instead of
"omap3-i2c@address". There is no recommended generic term for an
image signal processor, "isp" looks ok to me and seems to be
already used in NVIDIA Tegra's device tree files. So maybe:

isp@480bc000 {

> [...]

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [RFC 14/18] dt: bindings: Add bindings for omap3isp

[Laurent Pinchart]

Hi Sebastian,

Thank you for the review.

On Friday 13 March 2015 10:34:53 Sebastian Reichel wrote:
> On Fri, Mar 13, 2015 at 01:03:21AM +0200, Sakari Ailus wrote:
> > [...]
> > 
> > > > +Required properties
> > > > +===================
> > > > +
> > > > +compatible	: "ti,omap3-isp"
> > > 
> > > I would rephrase that using the usual wording as "compatible: Must
> > > contain
> > > "ti,omap3-isp".
> > 
> > [...]
> > 
> > > > +ti,phy-type	: 0 -- 3430; 1 -- 3630
> > > 
> > > Would it make sense to add #define's for this ?
> > 
> > I'll use OMAP3ISP_PHY_TYPE_COMPLEX_IO and OMAP3ISP_PHY_TYPE_CSIPHY as
> > discussed.
> > 
> > > It could also make sense to document/name them "Complex I/O" and
> > > "CSIPHY" to avoid referring to the SoC that implements them, as the ISP
> > > is also found in SoCs other than 3430 and 3630.
> > > 
> > > Could the PHY type be derived from the ES revision that we query at
> > > runtime ?
> >
> > I think this would work on 3430 and 3630 but I'm not certain about others.
> > 
> > > We should also take into account the fact that the DM3730 has officially
> > > no CSIPHY, but still seems to implement them in practice.
> > 
> > The DT sources are for 36xx, but I'd guess it works on 37xx as well,
> > doesn't it?
> 
> In other drivers this kind of information is often extracted from the
> compatible string. For example:
> 
> { .compatible = "ti,omap34xx-isp", .data = OMAP3ISP_PHY_TYPE_COMPLEX_IO, },
> { .compatible = "ti,omap36xx-isp", .data = OMAP3ISP_PHY_TYPE_CSIPHY, },
> ...

That's an option too, which I've discussed with Sakari before. The reason why 
we have decided to go for a separate property is that the PHY type seems to be 
more an SoC integration property than an ISP model property. I'm open to 
reconsidering that though.

Another option that has been discussed was to infer the PHY type from the ISP 
revision number queried at runtime. That would be fine for the 3430, 3630 and 
3730, but it remains unclear at this point whether this scheme would work with 
other SoCs. It should also be noted that some OMAP3-based SoCs that 
incorporate the ISP officially don't include the CSI PHYs, but seem to have 
them in practice.

> > [...]
> > 
> > > > +Example
> > > > +=======
> > > > +
> > > > +		omap3_isp: omap3_isp@480bc000 {
> > > 
> > > DT node names traditionally use - as a separator. Furthermore the
> > > phandle isn't needed. This should thus probably be
> > > 
> > > 	omap3-isp@480bc000 {
> > 
> > Fixed.
> 
> According to ePAPR this should be a generic name (page 19); For
> example the i2c node name should be "i2c@address" instead of
> "omap3-i2c@address". There is no recommended generic term for an
> image signal processor, "isp" looks ok to me and seems to be
> already used in NVIDIA Tegra's device tree files. So maybe:
> 
> isp@480bc000 {

"isp" sounds good to me.

-- 
Regards,

Laurent Pinchart

Re: [RFC 14/18] dt: bindings: Add bindings for omap3isp

[Sakari Ailus]

Hi Sebastian,

Thanks for the comments!

On Fri, Mar 13, 2015 at 10:34:53AM +0100, Sebastian Reichel wrote:
> Hi,
> 
> On Fri, Mar 13, 2015 at 01:03:21AM +0200, Sakari Ailus wrote:
> > [...]
> >
> > > > +Required properties
> > > > +===================
> > > > +
> > > > +compatible	: "ti,omap3-isp"
> > > 
> > > I would rephrase that using the usual wording as "compatible: Must contain 
> > > "ti,omap3-isp".
> >
> > [...]
> >
> > > > +ti,phy-type	: 0 -- 3430; 1 -- 3630
> > > 
> > > Would it make sense to add #define's for this ?
> > 
> > I'll use OMAP3ISP_PHY_TYPE_COMPLEX_IO and OMAP3ISP_PHY_TYPE_CSIPHY as
> > discussed.
> > 
> > > It could also make sense to document/name them "Complex I/O" and "CSIPHY" to 
> > > avoid referring to the SoC that implements them, as the ISP is also found in 
> > > SoCs other than 3430 and 3630.
> > > 
> > > Could the PHY type be derived from the ES revision that we query at runtime ?
> > 
> > I think this would work on 3430 and 3630 but I'm not certain about others.
> > 
> > > We should also take into account the fact that the DM3730 has officially no 
> > > CSIPHY, but still seems to implement them in practice.
> > 
> > The DT sources are for 36xx, but I'd guess it works on 37xx as well, doesn't
> > it?
> 
> In other drivers this kind of information is often extracted from the
> compatible string. For example:
> 
> { .compatible = "ti,omap34xx-isp", .data = OMAP3ISP_PHY_TYPE_COMPLEX_IO, },
> { .compatible = "ti,omap36xx-isp", .data = OMAP3ISP_PHY_TYPE_CSIPHY, },
> ...

As Laurent said, I'd prefer to keep it as it is now; they phy selection
isn't really a part of the ISP in hardware either. It just happens to be
that the first phy selection logic can be found in 3430 (isp rev 2.0) and
latter in 3630 (isp rev 15.0).

> > [...]
> >
> > > > +Example
> > > > +=======
> > > > +
> > > > +		omap3_isp: omap3_isp@480bc000 {
> > > 
> > > DT node names traditionally use - as a separator. Furthermore the phandle 
> > > isn't needed. This should thus probably be
> > > 
> > > 	omap3-isp@480bc000 {
> > 
> > Fixed.
> 
> According to ePAPR this should be a generic name (page 19); For
> example the i2c node name should be "i2c@address" instead of
> "omap3-i2c@address". There is no recommended generic term for an
> image signal processor, "isp" looks ok to me and seems to be
> already used in NVIDIA Tegra's device tree files. So maybe:
> 
> isp@480bc000 {

Thanks for the suggestion. I'll fix that.

-- 
Kind regards,

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

[RFC 15/18] 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       |  206 +++++++++++++++++++++++++--
 drivers/media/platform/omap3isp/isp.h       |   11 ++
 drivers/media/platform/omap3isp/ispcsiphy.c |    7 +
 3 files changed, 213 insertions(+), 11 deletions(-)

diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c
index 7804895..2c9bc0d 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,9 @@ static int isp_probe(struct platform_device *pdev)
 		goto error_isp;
 	}
 
+	if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node)
+		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,14 +2541,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");
-	isp->syscon_offset = isp_res_maps[m].syscon_offset;
-	isp->phy_type = isp_res_maps[m].phy_type;
-	if (IS_ERR(isp->syscon)) {
-		ret = PTR_ERR(isp->syscon);
-		goto error_isp;
-	}
-
 	/* IOMMU */
 	ret = isp_attach_iommu(isp);
 	if (ret < 0) {
@@ -2393,6 +2568,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;
@@ -2428,6 +2606,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,
@@ -2435,6 +2618,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: [RFC 15/18] omap3isp: Add support for the Device Tree

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Saturday 07 March 2015 23:41:12 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-X3B1VOXEql0@public.gmane.org>
> ---
>  drivers/media/platform/omap3isp/isp.c       |  206 ++++++++++++++++++++++--
>  drivers/media/platform/omap3isp/isp.h       |   11 ++
>  drivers/media/platform/omap3isp/ispcsiphy.c |    7 +
>  3 files changed, 213 insertions(+), 11 deletions(-)

[snip]

> @@ -2358,14 +2541,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");
> -	isp->syscon_offset = isp_res_maps[m].syscon_offset;

You're removing syscon_offset initialization here but not adding it anywhere 
else. This patch doesn't match the commit in your rm696-053-upstream branch, 
could you send the right version ? I'll then review it.

> -	isp->phy_type = isp_res_maps[m].phy_type;
> -	if (IS_ERR(isp->syscon)) {
> -		ret = PTR_ERR(isp->syscon);
> -		goto error_isp;
> -	}
> -
>  	/* IOMMU */
>  	ret = isp_attach_iommu(isp);
>  	if (ret < 0) {

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 15/18] omap3isp: Add support for the Device Tree

[Sakari Ailus]

Hi Laurent,

On Thu, Mar 12, 2015 at 01:48:02AM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Saturday 07 March 2015 23:41:12 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       |  206 ++++++++++++++++++++++--
> >  drivers/media/platform/omap3isp/isp.h       |   11 ++
> >  drivers/media/platform/omap3isp/ispcsiphy.c |    7 +
> >  3 files changed, 213 insertions(+), 11 deletions(-)
> 
> [snip]
> 
> > @@ -2358,14 +2541,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");
> > -	isp->syscon_offset = isp_res_maps[m].syscon_offset;
> 
> You're removing syscon_offset initialization here but not adding it anywhere 
> else. This patch doesn't match the commit in your rm696-053-upstream branch, 
> could you send the right version ? I'll then review it.

Yeah, there have been quite a few changes since I posted this RFC set, this
including. I'll post a new version once I've been able to take into account
all the comments I've got so far.

It'd be nice if someone could test the pdata support; I haven't had a chance
to do that in a few years now. :-)

-- 
Cheers,

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

[RFC 16/18] arm: dts: omap3: Add DT entries for OMAP 3

[Sakari Ailus]

The resources the ISP needs are slightly different on 3[45]xx and 3[67]xx.
Especially the phy-type property is different.

Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
---
 arch/arm/boot/dts/omap34xx.dtsi |   15 +++++++++++++++
 arch/arm/boot/dts/omap36xx.dtsi |   15 +++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/arch/arm/boot/dts/omap34xx.dtsi b/arch/arm/boot/dts/omap34xx.dtsi
index 3819c1e..4c034d0 100644
--- a/arch/arm/boot/dts/omap34xx.dtsi
+++ b/arch/arm/boot/dts/omap34xx.dtsi
@@ -37,6 +37,21 @@
 			pinctrl-single,register-width = <16>;
 			pinctrl-single,function-mask = <0xff1f>;
 		};
+
+		omap3_isp: omap3_isp@480bc000 {
+			compatible = "ti,omap3-isp";
+			reg = <0x480bc000 0x12fc
+			       0x480bd800 0x017c>;
+			interrupts = <24>;
+			iommus = <&mmu_isp>;
+			syscon = <&omap3_scm_general 0xdc>;
+			ti,phy-type = <0>;
+			#clock-cells = <1>;
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+			};
+		};
 	};
 };
 
diff --git a/arch/arm/boot/dts/omap36xx.dtsi b/arch/arm/boot/dts/omap36xx.dtsi
index 541704a..31ac41c 100644
--- a/arch/arm/boot/dts/omap36xx.dtsi
+++ b/arch/arm/boot/dts/omap36xx.dtsi
@@ -69,6 +69,21 @@
 			pinctrl-single,register-width = <16>;
 			pinctrl-single,function-mask = <0xff1f>;
 		};
+
+		omap3_isp: omap3_isp@480bc000 {
+			compatible = "ti,omap3-isp";
+			reg = <0x480bc000 0x12fc
+			       0x480bd800 0x0600>;
+			interrupts = <24>;
+			iommus = <&mmu_isp>;
+			syscon = <&omap3_scm_general 0x2f0>;
+			ti,phy-type = <1>;
+			#clock-cells = <1>;
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+			};
+		};
 	};
 };
 
-- 
1.7.10.4

Re: [RFC 16/18] arm: dts: omap3: Add DT entries for OMAP 3

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Saturday 07 March 2015 23:41:13 Sakari Ailus wrote:
> The resources the ISP needs are slightly different on 3[45]xx and 3[67]xx.
> Especially the phy-type property is different.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus-X3B1VOXEql0@public.gmane.org>
> ---
>  arch/arm/boot/dts/omap34xx.dtsi |   15 +++++++++++++++
>  arch/arm/boot/dts/omap36xx.dtsi |   15 +++++++++++++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/omap34xx.dtsi
> b/arch/arm/boot/dts/omap34xx.dtsi index 3819c1e..4c034d0 100644
> --- a/arch/arm/boot/dts/omap34xx.dtsi
> +++ b/arch/arm/boot/dts/omap34xx.dtsi
> @@ -37,6 +37,21 @@
>  			pinctrl-single,register-width = <16>;
>  			pinctrl-single,function-mask = <0xff1f>;
>  		};
> +
> +		omap3_isp: omap3_isp@480bc000 {
> +			compatible = "ti,omap3-isp";
> +			reg = <0x480bc000 0x12fc
> +			       0x480bd800 0x017c>;
> +			interrupts = <24>;
> +			iommus = <&mmu_isp>;
> +			syscon = <&omap3_scm_general 0xdc>;
> +			ti,phy-type = <0>;
> +			#clock-cells = <1>;
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;

How about predefining the ports too ?

> +			};
> +		};
>  	};
>  };
> 
> diff --git a/arch/arm/boot/dts/omap36xx.dtsi
> b/arch/arm/boot/dts/omap36xx.dtsi index 541704a..31ac41c 100644
> --- a/arch/arm/boot/dts/omap36xx.dtsi
> +++ b/arch/arm/boot/dts/omap36xx.dtsi
> @@ -69,6 +69,21 @@
>  			pinctrl-single,register-width = <16>;
>  			pinctrl-single,function-mask = <0xff1f>;
>  		};
> +
> +		omap3_isp: omap3_isp@480bc000 {
> +			compatible = "ti,omap3-isp";
> +			reg = <0x480bc000 0x12fc
> +			       0x480bd800 0x0600>;
> +			interrupts = <24>;
> +			iommus = <&mmu_isp>;
> +			syscon = <&omap3_scm_general 0x2f0>;
> +			ti,phy-type = <1>;
> +			#clock-cells = <1>;
> +			ports {
> +				#address-cells = <1>;
> +				#size-cells = <0>;

And obviously here too.

> +			};
> +		};
>  	};
>  };

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFC 16/18] arm: dts: omap3: Add DT entries for OMAP 3

[Sakari Ailus]

On Sun, Mar 08, 2015 at 01:51:51AM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Saturday 07 March 2015 23:41:13 Sakari Ailus wrote:
> > The resources the ISP needs are slightly different on 3[45]xx and 3[67]xx.
> > Especially the phy-type property is different.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@iki.fi>
> > ---
> >  arch/arm/boot/dts/omap34xx.dtsi |   15 +++++++++++++++
> >  arch/arm/boot/dts/omap36xx.dtsi |   15 +++++++++++++++
> >  2 files changed, 30 insertions(+)
> > 
> > diff --git a/arch/arm/boot/dts/omap34xx.dtsi
> > b/arch/arm/boot/dts/omap34xx.dtsi index 3819c1e..4c034d0 100644
> > --- a/arch/arm/boot/dts/omap34xx.dtsi
> > +++ b/arch/arm/boot/dts/omap34xx.dtsi
> > @@ -37,6 +37,21 @@
> >  			pinctrl-single,register-width = <16>;
> >  			pinctrl-single,function-mask = <0xff1f>;
> >  		};
> > +
> > +		omap3_isp: omap3_isp@480bc000 {
> > +			compatible = "ti,omap3-isp";
> > +			reg = <0x480bc000 0x12fc
> > +			       0x480bd800 0x017c>;
> > +			interrupts = <24>;
> > +			iommus = <&mmu_isp>;
> > +			syscon = <&omap3_scm_general 0xdc>;
> > +			ti,phy-type = <0>;
> > +			#clock-cells = <1>;
> > +			ports {
> > +				#address-cells = <1>;
> > +				#size-cells = <0>;
> 
> How about predefining the ports too ?

After a short discussion, we decided not to add port nodes. The arguments
considered were:

- Port nodes could help integrators writing the DT nodes. However the port
  nodes are easier to construct than endpoint nodes. Board specific
  configuration would also still need to be added.
- Extra port nodes take space which could be spent more usefully for other
  purposes.

-- 
Regards,

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