[PATCH 0/3] [media] atmel-isi: Add DT support for Atmel ISI driver

[PATCH 0/3] [media] atmel-isi: Add DT support for Atmel ISI driver

[Josh Wu]

This patch series add DT support for atmel ISI driver. It can support the
common v4l2 DT interfaces.

Josh Wu (3):
  [media] atmel-isi: add v4l2 async probe support
  [media] atmel-isi: convert the pdata from pointer to structure
  [media] atmel-isi: add primary DT support

 .../devicetree/bindings/media/atmel-isi.txt        |   51 +++++++++++++++++
 drivers/media/platform/soc_camera/atmel-isi.c      |   58 ++++++++++++++++----
 include/media/atmel-isi.h                          |    4 ++
 3 files changed, 101 insertions(+), 12 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/atmel-isi.txt

-- 
1.7.9.5

[PATCH 0/3] [media] atmel-isi: Add DT support for Atmel ISI driver

[Josh Wu]

This patch series add DT support for atmel ISI driver. It can support the
common v4l2 DT interfaces.

Josh Wu (3):
  [media] atmel-isi: add v4l2 async probe support
  [media] atmel-isi: convert the pdata from pointer to structure
  [media] atmel-isi: add primary DT support

 .../devicetree/bindings/media/atmel-isi.txt        |   51 +++++++++++++++++
 drivers/media/platform/soc_camera/atmel-isi.c      |   58 ++++++++++++++++----
 include/media/atmel-isi.h                          |    4 ++
 3 files changed, 101 insertions(+), 12 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/atmel-isi.txt

-- 
1.7.9.5

[PATCH 1/3] [media] atmel-isi: add v4l2 async probe support

[Josh Wu]

Signed-off-by: Josh Wu <josh.wu@atmel.com>
---
 drivers/media/platform/soc_camera/atmel-isi.c |    5 +++++
 include/media/atmel-isi.h                     |    4 ++++
 2 files changed, 9 insertions(+)

diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
index f0b6c90..d22aba1 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -982,6 +982,11 @@ static int atmel_isi_probe(struct platform_device *pdev)
 	soc_host->v4l2_dev.dev	= &pdev->dev;
 	soc_host->nr		= pdev->id;
 
+	if (isi->pdata.asd_sizes) {
+		soc_host->asd = isi->pdata.asd;
+		soc_host->asd_sizes = isi->pdata.asd_sizes;
+	}
+
 	ret = soc_camera_host_register(soc_host);
 	if (ret) {
 		dev_err(&pdev->dev, "Unable to register soc camera host\n");
diff --git a/include/media/atmel-isi.h b/include/media/atmel-isi.h
index 2b02347..c2e5703 100644
--- a/include/media/atmel-isi.h
+++ b/include/media/atmel-isi.h
@@ -106,6 +106,8 @@
 #define ISI_DATAWIDTH_8				0x01
 #define ISI_DATAWIDTH_10			0x02
 
+struct v4l2_async_subdev;
+
 struct isi_platform_data {
 	u8 has_emb_sync;
 	u8 emb_crc_sync;
@@ -118,6 +120,8 @@ struct isi_platform_data {
 	u32 frate;
 	/* Using for ISI_MCK */
 	u32 mck_hz;
+	struct v4l2_async_subdev **asd;	/* Flat array, arranged in groups */
+	int *asd_sizes;		/* 0-terminated array of asd group sizes */
 };
 
 #endif /* __ATMEL_ISI_H__ */
-- 
1.7.9.5

[PATCH 1/3] [media] atmel-isi: add v4l2 async probe support

[Josh Wu]

Signed-off-by: Josh Wu <josh.wu@atmel.com>
---
 drivers/media/platform/soc_camera/atmel-isi.c |    5 +++++
 include/media/atmel-isi.h                     |    4 ++++
 2 files changed, 9 insertions(+)

diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
index f0b6c90..d22aba1 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -982,6 +982,11 @@ static int atmel_isi_probe(struct platform_device *pdev)
 	soc_host->v4l2_dev.dev	= &pdev->dev;
 	soc_host->nr		= pdev->id;
 
+	if (isi->pdata.asd_sizes) {
+		soc_host->asd = isi->pdata.asd;
+		soc_host->asd_sizes = isi->pdata.asd_sizes;
+	}
+
 	ret = soc_camera_host_register(soc_host);
 	if (ret) {
 		dev_err(&pdev->dev, "Unable to register soc camera host\n");
diff --git a/include/media/atmel-isi.h b/include/media/atmel-isi.h
index 2b02347..c2e5703 100644
--- a/include/media/atmel-isi.h
+++ b/include/media/atmel-isi.h
@@ -106,6 +106,8 @@
 #define ISI_DATAWIDTH_8				0x01
 #define ISI_DATAWIDTH_10			0x02
 
+struct v4l2_async_subdev;
+
 struct isi_platform_data {
 	u8 has_emb_sync;
 	u8 emb_crc_sync;
@@ -118,6 +120,8 @@ struct isi_platform_data {
 	u32 frate;
 	/* Using for ISI_MCK */
 	u32 mck_hz;
+	struct v4l2_async_subdev **asd;	/* Flat array, arranged in groups */
+	int *asd_sizes;		/* 0-terminated array of asd group sizes */
 };
 
 #endif /* __ATMEL_ISI_H__ */
-- 
1.7.9.5

[PATCH 2/3] [media] atmel-isi: convert the pdata from pointer to structure

[Josh Wu]

Now the platform data is initialized by allocation of isi
structure. In the future, we use pdata to store the dt parameters.

Signed-off-by: Josh Wu <josh.wu@atmel.com>
---
 drivers/media/platform/soc_camera/atmel-isi.c |   22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
index d22aba1..93bf1cb 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -84,7 +84,7 @@ struct atmel_isi {
 	struct clk			*mck;
 	unsigned int			irq;
 
-	struct isi_platform_data	*pdata;
+	struct isi_platform_data	pdata;
 	u16				width_flags;	/* max 12 bits */
 
 	struct list_head		video_buffer_list;
@@ -350,7 +350,7 @@ static void start_dma(struct atmel_isi *isi, struct frame_buffer *buffer)
 
 	cfg1 &= ~ISI_CFG1_FRATE_DIV_MASK;
 	/* Enable linked list */
-	cfg1 |= isi->pdata->frate | ISI_CFG1_DISCR;
+	cfg1 |= isi->pdata.frate | ISI_CFG1_DISCR;
 
 	/* Enable codec path and ISI */
 	ctrl = ISI_CTRL_CDC | ISI_CTRL_EN;
@@ -797,7 +797,7 @@ static int isi_camera_set_bus_param(struct soc_camera_device *icd)
 	/* Make choises, based on platform preferences */
 	if ((common_flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH) &&
 	    (common_flags & V4L2_MBUS_HSYNC_ACTIVE_LOW)) {
-		if (isi->pdata->hsync_act_low)
+		if (isi->pdata.hsync_act_low)
 			common_flags &= ~V4L2_MBUS_HSYNC_ACTIVE_HIGH;
 		else
 			common_flags &= ~V4L2_MBUS_HSYNC_ACTIVE_LOW;
@@ -805,7 +805,7 @@ static int isi_camera_set_bus_param(struct soc_camera_device *icd)
 
 	if ((common_flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH) &&
 	    (common_flags & V4L2_MBUS_VSYNC_ACTIVE_LOW)) {
-		if (isi->pdata->vsync_act_low)
+		if (isi->pdata.vsync_act_low)
 			common_flags &= ~V4L2_MBUS_VSYNC_ACTIVE_HIGH;
 		else
 			common_flags &= ~V4L2_MBUS_VSYNC_ACTIVE_LOW;
@@ -813,7 +813,7 @@ static int isi_camera_set_bus_param(struct soc_camera_device *icd)
 
 	if ((common_flags & V4L2_MBUS_PCLK_SAMPLE_RISING) &&
 	    (common_flags & V4L2_MBUS_PCLK_SAMPLE_FALLING)) {
-		if (isi->pdata->pclk_act_falling)
+		if (isi->pdata.pclk_act_falling)
 			common_flags &= ~V4L2_MBUS_PCLK_SAMPLE_RISING;
 		else
 			common_flags &= ~V4L2_MBUS_PCLK_SAMPLE_FALLING;
@@ -835,9 +835,9 @@ static int isi_camera_set_bus_param(struct soc_camera_device *icd)
 	if (common_flags & V4L2_MBUS_PCLK_SAMPLE_FALLING)
 		cfg1 |= ISI_CFG1_PIXCLK_POL_ACTIVE_FALLING;
 
-	if (isi->pdata->has_emb_sync)
+	if (isi->pdata.has_emb_sync)
 		cfg1 |= ISI_CFG1_EMB_SYNC;
-	if (isi->pdata->full_mode)
+	if (isi->pdata.full_mode)
 		cfg1 |= ISI_CFG1_FULL_MODE;
 
 	isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
@@ -905,7 +905,7 @@ static int atmel_isi_probe(struct platform_device *pdev)
 	if (IS_ERR(isi->pclk))
 		return PTR_ERR(isi->pclk);
 
-	isi->pdata = pdata;
+	memcpy(&isi->pdata, pdata, sizeof(struct isi_platform_data));
 	isi->active = NULL;
 	spin_lock_init(&isi->lock);
 	INIT_LIST_HEAD(&isi->video_buffer_list);
@@ -921,7 +921,7 @@ static int atmel_isi_probe(struct platform_device *pdev)
 		/* Set ISI_MCK's frequency, it should be faster than pixel
 		 * clock.
 		 */
-		ret = clk_set_rate(isi->mck, pdata->mck_hz);
+		ret = clk_set_rate(isi->mck, isi->pdata.mck_hz);
 		if (ret < 0)
 			return ret;
 	}
@@ -955,9 +955,9 @@ static int atmel_isi_probe(struct platform_device *pdev)
 		goto err_ioremap;
 	}
 
-	if (pdata->data_width_flags & ISI_DATAWIDTH_8)
+	if (isi->pdata.data_width_flags & ISI_DATAWIDTH_8)
 		isi->width_flags = 1 << 7;
-	if (pdata->data_width_flags & ISI_DATAWIDTH_10)
+	if (isi->pdata.data_width_flags & ISI_DATAWIDTH_10)
 		isi->width_flags |= 1 << 9;
 
 	isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
-- 
1.7.9.5

[PATCH 2/3] [media] atmel-isi: convert the pdata from pointer to structure

[Josh Wu]

Now the platform data is initialized by allocation of isi
structure. In the future, we use pdata to store the dt parameters.

Signed-off-by: Josh Wu <josh.wu@atmel.com>
---
 drivers/media/platform/soc_camera/atmel-isi.c |   22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
index d22aba1..93bf1cb 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -84,7 +84,7 @@ struct atmel_isi {
 	struct clk			*mck;
 	unsigned int			irq;
 
-	struct isi_platform_data	*pdata;
+	struct isi_platform_data	pdata;
 	u16				width_flags;	/* max 12 bits */
 
 	struct list_head		video_buffer_list;
@@ -350,7 +350,7 @@ static void start_dma(struct atmel_isi *isi, struct frame_buffer *buffer)
 
 	cfg1 &= ~ISI_CFG1_FRATE_DIV_MASK;
 	/* Enable linked list */
-	cfg1 |= isi->pdata->frate | ISI_CFG1_DISCR;
+	cfg1 |= isi->pdata.frate | ISI_CFG1_DISCR;
 
 	/* Enable codec path and ISI */
 	ctrl = ISI_CTRL_CDC | ISI_CTRL_EN;
@@ -797,7 +797,7 @@ static int isi_camera_set_bus_param(struct soc_camera_device *icd)
 	/* Make choises, based on platform preferences */
 	if ((common_flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH) &&
 	    (common_flags & V4L2_MBUS_HSYNC_ACTIVE_LOW)) {
-		if (isi->pdata->hsync_act_low)
+		if (isi->pdata.hsync_act_low)
 			common_flags &= ~V4L2_MBUS_HSYNC_ACTIVE_HIGH;
 		else
 			common_flags &= ~V4L2_MBUS_HSYNC_ACTIVE_LOW;
@@ -805,7 +805,7 @@ static int isi_camera_set_bus_param(struct soc_camera_device *icd)
 
 	if ((common_flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH) &&
 	    (common_flags & V4L2_MBUS_VSYNC_ACTIVE_LOW)) {
-		if (isi->pdata->vsync_act_low)
+		if (isi->pdata.vsync_act_low)
 			common_flags &= ~V4L2_MBUS_VSYNC_ACTIVE_HIGH;
 		else
 			common_flags &= ~V4L2_MBUS_VSYNC_ACTIVE_LOW;
@@ -813,7 +813,7 @@ static int isi_camera_set_bus_param(struct soc_camera_device *icd)
 
 	if ((common_flags & V4L2_MBUS_PCLK_SAMPLE_RISING) &&
 	    (common_flags & V4L2_MBUS_PCLK_SAMPLE_FALLING)) {
-		if (isi->pdata->pclk_act_falling)
+		if (isi->pdata.pclk_act_falling)
 			common_flags &= ~V4L2_MBUS_PCLK_SAMPLE_RISING;
 		else
 			common_flags &= ~V4L2_MBUS_PCLK_SAMPLE_FALLING;
@@ -835,9 +835,9 @@ static int isi_camera_set_bus_param(struct soc_camera_device *icd)
 	if (common_flags & V4L2_MBUS_PCLK_SAMPLE_FALLING)
 		cfg1 |= ISI_CFG1_PIXCLK_POL_ACTIVE_FALLING;
 
-	if (isi->pdata->has_emb_sync)
+	if (isi->pdata.has_emb_sync)
 		cfg1 |= ISI_CFG1_EMB_SYNC;
-	if (isi->pdata->full_mode)
+	if (isi->pdata.full_mode)
 		cfg1 |= ISI_CFG1_FULL_MODE;
 
 	isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
@@ -905,7 +905,7 @@ static int atmel_isi_probe(struct platform_device *pdev)
 	if (IS_ERR(isi->pclk))
 		return PTR_ERR(isi->pclk);
 
-	isi->pdata = pdata;
+	memcpy(&isi->pdata, pdata, sizeof(struct isi_platform_data));
 	isi->active = NULL;
 	spin_lock_init(&isi->lock);
 	INIT_LIST_HEAD(&isi->video_buffer_list);
@@ -921,7 +921,7 @@ static int atmel_isi_probe(struct platform_device *pdev)
 		/* Set ISI_MCK's frequency, it should be faster than pixel
 		 * clock.
 		 */
-		ret = clk_set_rate(isi->mck, pdata->mck_hz);
+		ret = clk_set_rate(isi->mck, isi->pdata.mck_hz);
 		if (ret < 0)
 			return ret;
 	}
@@ -955,9 +955,9 @@ static int atmel_isi_probe(struct platform_device *pdev)
 		goto err_ioremap;
 	}
 
-	if (pdata->data_width_flags & ISI_DATAWIDTH_8)
+	if (isi->pdata.data_width_flags & ISI_DATAWIDTH_8)
 		isi->width_flags = 1 << 7;
-	if (pdata->data_width_flags & ISI_DATAWIDTH_10)
+	if (isi->pdata.data_width_flags & ISI_DATAWIDTH_10)
 		isi->width_flags |= 1 << 9;
 
 	isi_writel(isi, ISI_CTRL, ISI_CTRL_DIS);
-- 
1.7.9.5

[PATCH 3/3] [media] atmel-isi: add primary DT support

[Josh Wu]

This patch add the DT support for Atmel ISI driver.
It use the same v4l2 DT interface that defined in video-interfaces.txt.

Signed-off-by: Josh Wu <josh.wu@atmel.com>
Cc: devicetree at vger.kernel.org
---
 .../devicetree/bindings/media/atmel-isi.txt        |   51 ++++++++++++++++++++
 drivers/media/platform/soc_camera/atmel-isi.c      |   33 ++++++++++++-
 2 files changed, 82 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/atmel-isi.txt

diff --git a/Documentation/devicetree/bindings/media/atmel-isi.txt b/Documentation/devicetree/bindings/media/atmel-isi.txt
new file mode 100644
index 0000000..07f00eb
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/atmel-isi.txt
@@ -0,0 +1,51 @@
+Atmel Image Sensor Interface (ISI) SoC Camera Subsystem
+----------------------------------------------
+
+Required properties:
+- compatible: must be "atmel,at91sam9g45-isi"
+- reg: physical base address and length of the registers set for the device;
+- interrupts: should contain IRQ line for the ISI;
+- clocks: list of clock specifiers, corresponding to entries in
+          the clock-names property;
+- clock-names: must contain "isi_clk", which is the isi peripherial clock.
+               "isi_mck" is optinal, it is the master clock output to sensor.
+
+Optional properties:
+- atmel,isi-disable-preview: a boolean property to disable the preview channel;
+
+Example:
+	isi: isi at f0034000 {
+		compatible = "atmel,at91sam9g45-isi";
+		reg = <0xf0034000 0x4000>;
+		interrupts = <37 IRQ_TYPE_LEVEL_HIGH 5>;
+
+		clocks = <&isi_clk>, <&pck1>;
+		clock-names = "isi_clk", "isi_mck";
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_isi &pinctrl_pck1_as_isi_mck>;
+
+		port {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			isi_0: endpoint {
+				remote-endpoint = <&ov2640_0>;
+			};
+		};
+	};
+
+	i2c1: i2c at f0018000 {
+		ov2640: camera at 0x30 {
+			compatible = "omnivision,ov2640";
+			reg = <0x30>;
+
+			port {
+				ov2640_0: endpoint {
+					remote-endpoint = <&isi_0>;
+					bus-width = <8>;
+				};
+			};
+		};
+	};
+
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
index 93bf1cb..1822129 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -19,6 +19,7 @@
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 
@@ -33,6 +34,7 @@
 #define VID_LIMIT_BYTES			(16 * 1024 * 1024)
 #define MIN_FRAME_RATE			15
 #define FRAME_INTERVAL_MILLI_SEC	(1000 / MIN_FRAME_RATE)
+#define ISI_DEFAULT_MCLK_FREQ		25000000
 
 /* Frame buffer descriptor */
 struct fbd {
@@ -878,6 +880,22 @@ static int atmel_isi_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static int atmel_isi_probe_dt(struct atmel_isi *isi,
+			struct platform_device *pdev)
+{
+	struct device_node *node = pdev->dev.of_node;
+
+	isi->pdata.full_mode = !of_property_read_bool(node,
+			"atmel,isi-disable-preview");
+
+	/* Default settings for ISI */
+	isi->pdata.mck_hz = ISI_DEFAULT_MCLK_FREQ;
+	isi->pdata.frate = ISI_CFG1_FRATE_CAPTURE_ALL;
+	isi->pdata.data_width_flags = ISI_DATAWIDTH_8 | ISI_DATAWIDTH_10;
+
+	return 0;
+}
+
 static int atmel_isi_probe(struct platform_device *pdev)
 {
 	unsigned int irq;
@@ -889,7 +907,7 @@ static int atmel_isi_probe(struct platform_device *pdev)
 	struct isi_platform_data *pdata;
 
 	pdata = dev->platform_data;
-	if (!pdata || !pdata->data_width_flags) {
+	if ((!pdata || !pdata->data_width_flags) && !pdev->dev.of_node) {
 		dev_err(&pdev->dev,
 			"No config available for Atmel ISI\n");
 		return -EINVAL;
@@ -905,7 +923,11 @@ static int atmel_isi_probe(struct platform_device *pdev)
 	if (IS_ERR(isi->pclk))
 		return PTR_ERR(isi->pclk);
 
-	memcpy(&isi->pdata, pdata, sizeof(struct isi_platform_data));
+	if (pdata)
+		memcpy(&isi->pdata, pdata, sizeof(struct isi_platform_data));
+	else	/* dt probe */
+		atmel_isi_probe_dt(isi, pdev);
+
 	isi->active = NULL;
 	spin_lock_init(&isi->lock);
 	INIT_LIST_HEAD(&isi->video_buffer_list);
@@ -1007,11 +1029,18 @@ err_alloc_ctx:
 	return ret;
 }
 
+static const struct of_device_id atmel_isi_of_match[] = {
+	{ .compatible = "atmel,at91sam9g45-isi" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, atmel_isi_of_match);
+
 static struct platform_driver atmel_isi_driver = {
 	.remove		= atmel_isi_remove,
 	.driver		= {
 		.name = "atmel_isi",
 		.owner = THIS_MODULE,
+		.of_match_table = atmel_isi_of_match,
 	},
 };
 
-- 
1.7.9.5

[PATCH 3/3] [media] atmel-isi: add primary DT support

[Josh Wu]

This patch add the DT support for Atmel ISI driver.
It use the same v4l2 DT interface that defined in video-interfaces.txt.

Signed-off-by: Josh Wu <josh.wu@atmel.com>
Cc: devicetree@vger.kernel.org
---
 .../devicetree/bindings/media/atmel-isi.txt        |   51 ++++++++++++++++++++
 drivers/media/platform/soc_camera/atmel-isi.c      |   33 ++++++++++++-
 2 files changed, 82 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/atmel-isi.txt

diff --git a/Documentation/devicetree/bindings/media/atmel-isi.txt b/Documentation/devicetree/bindings/media/atmel-isi.txt
new file mode 100644
index 0000000..07f00eb
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/atmel-isi.txt
@@ -0,0 +1,51 @@
+Atmel Image Sensor Interface (ISI) SoC Camera Subsystem
+----------------------------------------------
+
+Required properties:
+- compatible: must be "atmel,at91sam9g45-isi"
+- reg: physical base address and length of the registers set for the device;
+- interrupts: should contain IRQ line for the ISI;
+- clocks: list of clock specifiers, corresponding to entries in
+          the clock-names property;
+- clock-names: must contain "isi_clk", which is the isi peripherial clock.
+               "isi_mck" is optinal, it is the master clock output to sensor.
+
+Optional properties:
+- atmel,isi-disable-preview: a boolean property to disable the preview channel;
+
+Example:
+	isi: isi@f0034000 {
+		compatible = "atmel,at91sam9g45-isi";
+		reg = <0xf0034000 0x4000>;
+		interrupts = <37 IRQ_TYPE_LEVEL_HIGH 5>;
+
+		clocks = <&isi_clk>, <&pck1>;
+		clock-names = "isi_clk", "isi_mck";
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_isi &pinctrl_pck1_as_isi_mck>;
+
+		port {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			isi_0: endpoint {
+				remote-endpoint = <&ov2640_0>;
+			};
+		};
+	};
+
+	i2c1: i2c@f0018000 {
+		ov2640: camera@0x30 {
+			compatible = "omnivision,ov2640";
+			reg = <0x30>;
+
+			port {
+				ov2640_0: endpoint {
+					remote-endpoint = <&isi_0>;
+					bus-width = <8>;
+				};
+			};
+		};
+	};
+
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
index 93bf1cb..1822129 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -19,6 +19,7 @@
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 
@@ -33,6 +34,7 @@
 #define VID_LIMIT_BYTES			(16 * 1024 * 1024)
 #define MIN_FRAME_RATE			15
 #define FRAME_INTERVAL_MILLI_SEC	(1000 / MIN_FRAME_RATE)
+#define ISI_DEFAULT_MCLK_FREQ		25000000
 
 /* Frame buffer descriptor */
 struct fbd {
@@ -878,6 +880,22 @@ static int atmel_isi_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static int atmel_isi_probe_dt(struct atmel_isi *isi,
+			struct platform_device *pdev)
+{
+	struct device_node *node = pdev->dev.of_node;
+
+	isi->pdata.full_mode = !of_property_read_bool(node,
+			"atmel,isi-disable-preview");
+
+	/* Default settings for ISI */
+	isi->pdata.mck_hz = ISI_DEFAULT_MCLK_FREQ;
+	isi->pdata.frate = ISI_CFG1_FRATE_CAPTURE_ALL;
+	isi->pdata.data_width_flags = ISI_DATAWIDTH_8 | ISI_DATAWIDTH_10;
+
+	return 0;
+}
+
 static int atmel_isi_probe(struct platform_device *pdev)
 {
 	unsigned int irq;
@@ -889,7 +907,7 @@ static int atmel_isi_probe(struct platform_device *pdev)
 	struct isi_platform_data *pdata;
 
 	pdata = dev->platform_data;
-	if (!pdata || !pdata->data_width_flags) {
+	if ((!pdata || !pdata->data_width_flags) && !pdev->dev.of_node) {
 		dev_err(&pdev->dev,
 			"No config available for Atmel ISI\n");
 		return -EINVAL;
@@ -905,7 +923,11 @@ static int atmel_isi_probe(struct platform_device *pdev)
 	if (IS_ERR(isi->pclk))
 		return PTR_ERR(isi->pclk);
 
-	memcpy(&isi->pdata, pdata, sizeof(struct isi_platform_data));
+	if (pdata)
+		memcpy(&isi->pdata, pdata, sizeof(struct isi_platform_data));
+	else	/* dt probe */
+		atmel_isi_probe_dt(isi, pdev);
+
 	isi->active = NULL;
 	spin_lock_init(&isi->lock);
 	INIT_LIST_HEAD(&isi->video_buffer_list);
@@ -1007,11 +1029,18 @@ err_alloc_ctx:
 	return ret;
 }
 
+static const struct of_device_id atmel_isi_of_match[] = {
+	{ .compatible = "atmel,at91sam9g45-isi" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, atmel_isi_of_match);
+
 static struct platform_driver atmel_isi_driver = {
 	.remove		= atmel_isi_remove,
 	.driver		= {
 		.name = "atmel_isi",
 		.owner = THIS_MODULE,
+		.of_match_table = atmel_isi_of_match,
 	},
 };
 
-- 
1.7.9.5

[PATCH 3/3] [media] atmel-isi: add primary DT support

[Josh Wu]

This patch add the DT support for Atmel ISI driver.
It use the same v4l2 DT interface that defined in video-interfaces.txt.

Signed-off-by: Josh Wu <josh.wu@atmel.com>
Cc: devicetree@vger.kernel.org
---
 .../devicetree/bindings/media/atmel-isi.txt        |   51 ++++++++++++++++++++
 drivers/media/platform/soc_camera/atmel-isi.c      |   33 ++++++++++++-
 2 files changed, 82 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/atmel-isi.txt

diff --git a/Documentation/devicetree/bindings/media/atmel-isi.txt b/Documentation/devicetree/bindings/media/atmel-isi.txt
new file mode 100644
index 0000000..07f00eb
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/atmel-isi.txt
@@ -0,0 +1,51 @@
+Atmel Image Sensor Interface (ISI) SoC Camera Subsystem
+----------------------------------------------
+
+Required properties:
+- compatible: must be "atmel,at91sam9g45-isi"
+- reg: physical base address and length of the registers set for the device;
+- interrupts: should contain IRQ line for the ISI;
+- clocks: list of clock specifiers, corresponding to entries in
+          the clock-names property;
+- clock-names: must contain "isi_clk", which is the isi peripherial clock.
+               "isi_mck" is optinal, it is the master clock output to sensor.
+
+Optional properties:
+- atmel,isi-disable-preview: a boolean property to disable the preview channel;
+
+Example:
+	isi: isi@f0034000 {
+		compatible = "atmel,at91sam9g45-isi";
+		reg = <0xf0034000 0x4000>;
+		interrupts = <37 IRQ_TYPE_LEVEL_HIGH 5>;
+
+		clocks = <&isi_clk>, <&pck1>;
+		clock-names = "isi_clk", "isi_mck";
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&pinctrl_isi &pinctrl_pck1_as_isi_mck>;
+
+		port {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			isi_0: endpoint {
+				remote-endpoint = <&ov2640_0>;
+			};
+		};
+	};
+
+	i2c1: i2c@f0018000 {
+		ov2640: camera@0x30 {
+			compatible = "omnivision,ov2640";
+			reg = <0x30>;
+
+			port {
+				ov2640_0: endpoint {
+					remote-endpoint = <&isi_0>;
+					bus-width = <8>;
+				};
+			};
+		};
+	};
+
diff --git a/drivers/media/platform/soc_camera/atmel-isi.c b/drivers/media/platform/soc_camera/atmel-isi.c
index 93bf1cb..1822129 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -19,6 +19,7 @@
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 
@@ -33,6 +34,7 @@
 #define VID_LIMIT_BYTES			(16 * 1024 * 1024)
 #define MIN_FRAME_RATE			15
 #define FRAME_INTERVAL_MILLI_SEC	(1000 / MIN_FRAME_RATE)
+#define ISI_DEFAULT_MCLK_FREQ		25000000
 
 /* Frame buffer descriptor */
 struct fbd {
@@ -878,6 +880,22 @@ static int atmel_isi_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static int atmel_isi_probe_dt(struct atmel_isi *isi,
+			struct platform_device *pdev)
+{
+	struct device_node *node = pdev->dev.of_node;
+
+	isi->pdata.full_mode = !of_property_read_bool(node,
+			"atmel,isi-disable-preview");
+
+	/* Default settings for ISI */
+	isi->pdata.mck_hz = ISI_DEFAULT_MCLK_FREQ;
+	isi->pdata.frate = ISI_CFG1_FRATE_CAPTURE_ALL;
+	isi->pdata.data_width_flags = ISI_DATAWIDTH_8 | ISI_DATAWIDTH_10;
+
+	return 0;
+}
+
 static int atmel_isi_probe(struct platform_device *pdev)
 {
 	unsigned int irq;
@@ -889,7 +907,7 @@ static int atmel_isi_probe(struct platform_device *pdev)
 	struct isi_platform_data *pdata;
 
 	pdata = dev->platform_data;
-	if (!pdata || !pdata->data_width_flags) {
+	if ((!pdata || !pdata->data_width_flags) && !pdev->dev.of_node) {
 		dev_err(&pdev->dev,
 			"No config available for Atmel ISI\n");
 		return -EINVAL;
@@ -905,7 +923,11 @@ static int atmel_isi_probe(struct platform_device *pdev)
 	if (IS_ERR(isi->pclk))
 		return PTR_ERR(isi->pclk);
 
-	memcpy(&isi->pdata, pdata, sizeof(struct isi_platform_data));
+	if (pdata)
+		memcpy(&isi->pdata, pdata, sizeof(struct isi_platform_data));
+	else	/* dt probe */
+		atmel_isi_probe_dt(isi, pdev);
+
 	isi->active = NULL;
 	spin_lock_init(&isi->lock);
 	INIT_LIST_HEAD(&isi->video_buffer_list);
@@ -1007,11 +1029,18 @@ err_alloc_ctx:
 	return ret;
 }
 
+static const struct of_device_id atmel_isi_of_match[] = {
+	{ .compatible = "atmel,at91sam9g45-isi" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, atmel_isi_of_match);
+
 static struct platform_driver atmel_isi_driver = {
 	.remove		= atmel_isi_remove,
 	.driver		= {
 		.name = "atmel_isi",
 		.owner = THIS_MODULE,
+		.of_match_table = atmel_isi_of_match,
 	},
 };
 
-- 
1.7.9.5

[PATCH 3/3] [media] atmel-isi: add primary DT support

[Laurent Pinchart]

Hi Josh,

Thank you for the patch.

On Tuesday 18 March 2014 19:19:54 Josh Wu wrote:
> This patch add the DT support for Atmel ISI driver.
> It use the same v4l2 DT interface that defined in video-interfaces.txt.
> 
> Signed-off-by: Josh Wu <josh.wu@atmel.com>
> Cc: devicetree at vger.kernel.org
> ---
>  .../devicetree/bindings/media/atmel-isi.txt        |   51 +++++++++++++++++
>  drivers/media/platform/soc_camera/atmel-isi.c      |   33 ++++++++++++-
>  2 files changed, 82 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/media/atmel-isi.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/atmel-isi.txt
> b/Documentation/devicetree/bindings/media/atmel-isi.txt new file mode
> 100644
> index 0000000..07f00eb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/atmel-isi.txt
> @@ -0,0 +1,51 @@
> +Atmel Image Sensor Interface (ISI) SoC Camera Subsystem
> +----------------------------------------------
> +
> +Required properties:
> +- compatible: must be "atmel,at91sam9g45-isi"
> +- reg: physical base address and length of the registers set for the
> device;
> +- interrupts: should contain IRQ line for the ISI;
> +- clocks: list of clock specifiers, corresponding to entries in
> +          the clock-names property;
> +- clock-names: must contain "isi_clk", which is the isi peripherial clock.
> +               "isi_mck" is optinal, it is the master clock output to
> sensor.

The mck clock should be handled by the sensor driver instead. I know we have a 
legacy mode in the atmel-isi driver to manage that clock internally, but let's 
not propagate that to DT. I would also drop the "isi_" prefix from the isi_clk 
name.

You should also describe the port node. You can just mention the related 
bindings document, and state that the ISI has a single port.

> +Optional properties:
> +- atmel,isi-disable-preview: a boolean property to disable the preview
> channel;

That doesn't really sound like a hardware property to me. Isn't it full mode 
related to software configuration instead, which should be performed at 
runtime by userspace ?

> +
> +Example:
> +	isi: isi at f0034000 {
> +		compatible = "atmel,at91sam9g45-isi";
> +		reg = <0xf0034000 0x4000>;
> +		interrupts = <37 IRQ_TYPE_LEVEL_HIGH 5>;
> +
> +		clocks = <&isi_clk>, <&pck1>;
> +		clock-names = "isi_clk", "isi_mck";
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_isi &pinctrl_pck1_as_isi_mck>;
> +
> +		port {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			isi_0: endpoint {
> +				remote-endpoint = <&ov2640_0>;
> +			};
> +		};
> +	};
> +
> +	i2c1: i2c at f0018000 {
> +		ov2640: camera at 0x30 {
> +			compatible = "omnivision,ov2640";
> +			reg = <0x30>;
> +
> +			port {
> +				ov2640_0: endpoint {
> +					remote-endpoint = <&isi_0>;
> +					bus-width = <8>;
> +				};
> +			};
> +		};
> +	};
> +
> diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
> b/drivers/media/platform/soc_camera/atmel-isi.c index 93bf1cb..1822129
> 100644
> --- a/drivers/media/platform/soc_camera/atmel-isi.c
> +++ b/drivers/media/platform/soc_camera/atmel-isi.c
> @@ -19,6 +19,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> +#include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
> 
> @@ -33,6 +34,7 @@
>  #define VID_LIMIT_BYTES			(16 * 1024 * 1024)
>  #define MIN_FRAME_RATE			15
>  #define FRAME_INTERVAL_MILLI_SEC	(1000 / MIN_FRAME_RATE)
> +#define ISI_DEFAULT_MCLK_FREQ		25000000
> 
>  /* Frame buffer descriptor */
>  struct fbd {
> @@ -878,6 +880,22 @@ static int atmel_isi_remove(struct platform_device
> *pdev) return 0;
>  }
> 
> +static int atmel_isi_probe_dt(struct atmel_isi *isi,
> +			struct platform_device *pdev)
> +{
> +	struct device_node *node = pdev->dev.of_node;
> +
> +	isi->pdata.full_mode = !of_property_read_bool(node,
> +			"atmel,isi-disable-preview");
> +
> +	/* Default settings for ISI */
> +	isi->pdata.mck_hz = ISI_DEFAULT_MCLK_FREQ;
> +	isi->pdata.frate = ISI_CFG1_FRATE_CAPTURE_ALL;
> +	isi->pdata.data_width_flags = ISI_DATAWIDTH_8 | ISI_DATAWIDTH_10;
> +
> +	return 0;
> +}
> +
>  static int atmel_isi_probe(struct platform_device *pdev)
>  {
>  	unsigned int irq;
> @@ -889,7 +907,7 @@ static int atmel_isi_probe(struct platform_device *pdev)
> struct isi_platform_data *pdata;
> 
>  	pdata = dev->platform_data;
> -	if (!pdata || !pdata->data_width_flags) {
> +	if ((!pdata || !pdata->data_width_flags) && !pdev->dev.of_node) {
>  		dev_err(&pdev->dev,
>  			"No config available for Atmel ISI\n");
>  		return -EINVAL;
> @@ -905,7 +923,11 @@ static int atmel_isi_probe(struct platform_device
> *pdev) if (IS_ERR(isi->pclk))
>  		return PTR_ERR(isi->pclk);
> 
> -	memcpy(&isi->pdata, pdata, sizeof(struct isi_platform_data));
> +	if (pdata)
> +		memcpy(&isi->pdata, pdata, sizeof(struct isi_platform_data));
> +	else	/* dt probe */
> +		atmel_isi_probe_dt(isi, pdev);
> +
>  	isi->active = NULL;
>  	spin_lock_init(&isi->lock);
>  	INIT_LIST_HEAD(&isi->video_buffer_list);
> @@ -1007,11 +1029,18 @@ err_alloc_ctx:
>  	return ret;
>  }
> 
> +static const struct of_device_id atmel_isi_of_match[] = {
> +	{ .compatible = "atmel,at91sam9g45-isi" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, atmel_isi_of_match);
> +
>  static struct platform_driver atmel_isi_driver = {
>  	.remove		= atmel_isi_remove,
>  	.driver		= {
>  		.name = "atmel_isi",
>  		.owner = THIS_MODULE,
> +		.of_match_table = atmel_isi_of_match,
>  	},
>  };

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/3] [media] atmel-isi: add primary DT support

[Laurent Pinchart]

Hi Josh,

Thank you for the patch.

On Tuesday 18 March 2014 19:19:54 Josh Wu wrote:
> This patch add the DT support for Atmel ISI driver.
> It use the same v4l2 DT interface that defined in video-interfaces.txt.
> 
> Signed-off-by: Josh Wu <josh.wu@atmel.com>
> Cc: devicetree@vger.kernel.org
> ---
>  .../devicetree/bindings/media/atmel-isi.txt        |   51 +++++++++++++++++
>  drivers/media/platform/soc_camera/atmel-isi.c      |   33 ++++++++++++-
>  2 files changed, 82 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/media/atmel-isi.txt
> 
> diff --git a/Documentation/devicetree/bindings/media/atmel-isi.txt
> b/Documentation/devicetree/bindings/media/atmel-isi.txt new file mode
> 100644
> index 0000000..07f00eb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/atmel-isi.txt
> @@ -0,0 +1,51 @@
> +Atmel Image Sensor Interface (ISI) SoC Camera Subsystem
> +----------------------------------------------
> +
> +Required properties:
> +- compatible: must be "atmel,at91sam9g45-isi"
> +- reg: physical base address and length of the registers set for the
> device;
> +- interrupts: should contain IRQ line for the ISI;
> +- clocks: list of clock specifiers, corresponding to entries in
> +          the clock-names property;
> +- clock-names: must contain "isi_clk", which is the isi peripherial clock.
> +               "isi_mck" is optinal, it is the master clock output to
> sensor.

The mck clock should be handled by the sensor driver instead. I know we have a 
legacy mode in the atmel-isi driver to manage that clock internally, but let's 
not propagate that to DT. I would also drop the "isi_" prefix from the isi_clk 
name.

You should also describe the port node. You can just mention the related 
bindings document, and state that the ISI has a single port.

> +Optional properties:
> +- atmel,isi-disable-preview: a boolean property to disable the preview
> channel;

That doesn't really sound like a hardware property to me. Isn't it full mode 
related to software configuration instead, which should be performed at 
runtime by userspace ?

> +
> +Example:
> +	isi: isi@f0034000 {
> +		compatible = "atmel,at91sam9g45-isi";
> +		reg = <0xf0034000 0x4000>;
> +		interrupts = <37 IRQ_TYPE_LEVEL_HIGH 5>;
> +
> +		clocks = <&isi_clk>, <&pck1>;
> +		clock-names = "isi_clk", "isi_mck";
> +
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&pinctrl_isi &pinctrl_pck1_as_isi_mck>;
> +
> +		port {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			isi_0: endpoint {
> +				remote-endpoint = <&ov2640_0>;
> +			};
> +		};
> +	};
> +
> +	i2c1: i2c@f0018000 {
> +		ov2640: camera@0x30 {
> +			compatible = "omnivision,ov2640";
> +			reg = <0x30>;
> +
> +			port {
> +				ov2640_0: endpoint {
> +					remote-endpoint = <&isi_0>;
> +					bus-width = <8>;
> +				};
> +			};
> +		};
> +	};
> +
> diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
> b/drivers/media/platform/soc_camera/atmel-isi.c index 93bf1cb..1822129
> 100644
> --- a/drivers/media/platform/soc_camera/atmel-isi.c
> +++ b/drivers/media/platform/soc_camera/atmel-isi.c
> @@ -19,6 +19,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> +#include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
> 
> @@ -33,6 +34,7 @@
>  #define VID_LIMIT_BYTES			(16 * 1024 * 1024)
>  #define MIN_FRAME_RATE			15
>  #define FRAME_INTERVAL_MILLI_SEC	(1000 / MIN_FRAME_RATE)
> +#define ISI_DEFAULT_MCLK_FREQ		25000000
> 
>  /* Frame buffer descriptor */
>  struct fbd {
> @@ -878,6 +880,22 @@ static int atmel_isi_remove(struct platform_device
> *pdev) return 0;
>  }
> 
> +static int atmel_isi_probe_dt(struct atmel_isi *isi,
> +			struct platform_device *pdev)
> +{
> +	struct device_node *node = pdev->dev.of_node;
> +
> +	isi->pdata.full_mode = !of_property_read_bool(node,
> +			"atmel,isi-disable-preview");
> +
> +	/* Default settings for ISI */
> +	isi->pdata.mck_hz = ISI_DEFAULT_MCLK_FREQ;
> +	isi->pdata.frate = ISI_CFG1_FRATE_CAPTURE_ALL;
> +	isi->pdata.data_width_flags = ISI_DATAWIDTH_8 | ISI_DATAWIDTH_10;
> +
> +	return 0;
> +}
> +
>  static int atmel_isi_probe(struct platform_device *pdev)
>  {
>  	unsigned int irq;
> @@ -889,7 +907,7 @@ static int atmel_isi_probe(struct platform_device *pdev)
> struct isi_platform_data *pdata;
> 
>  	pdata = dev->platform_data;
> -	if (!pdata || !pdata->data_width_flags) {
> +	if ((!pdata || !pdata->data_width_flags) && !pdev->dev.of_node) {
>  		dev_err(&pdev->dev,
>  			"No config available for Atmel ISI\n");
>  		return -EINVAL;
> @@ -905,7 +923,11 @@ static int atmel_isi_probe(struct platform_device
> *pdev) if (IS_ERR(isi->pclk))
>  		return PTR_ERR(isi->pclk);
> 
> -	memcpy(&isi->pdata, pdata, sizeof(struct isi_platform_data));
> +	if (pdata)
> +		memcpy(&isi->pdata, pdata, sizeof(struct isi_platform_data));
> +	else	/* dt probe */
> +		atmel_isi_probe_dt(isi, pdev);
> +
>  	isi->active = NULL;
>  	spin_lock_init(&isi->lock);
>  	INIT_LIST_HEAD(&isi->video_buffer_list);
> @@ -1007,11 +1029,18 @@ err_alloc_ctx:
>  	return ret;
>  }
> 
> +static const struct of_device_id atmel_isi_of_match[] = {
> +	{ .compatible = "atmel,at91sam9g45-isi" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, atmel_isi_of_match);
> +
>  static struct platform_driver atmel_isi_driver = {
>  	.remove		= atmel_isi_remove,
>  	.driver		= {
>  		.name = "atmel_isi",
>  		.owner = THIS_MODULE,
> +		.of_match_table = atmel_isi_of_match,
>  	},
>  };

-- 
Regards,

Laurent Pinchart

[PATCH 3/3] [media] atmel-isi: add primary DT support

[Josh Wu]

Hi, Laurent

On 3/18/2014 9:36 PM, Laurent Pinchart wrote:
> Hi Josh,
>
> Thank you for the patch.

Thanks for your review.

>
> On Tuesday 18 March 2014 19:19:54 Josh Wu wrote:
>> This patch add the DT support for Atmel ISI driver.
>> It use the same v4l2 DT interface that defined in video-interfaces.txt.
>>
>> Signed-off-by: Josh Wu <josh.wu@atmel.com>
>> Cc: devicetree at vger.kernel.org
>> ---
>>   .../devicetree/bindings/media/atmel-isi.txt        |   51 +++++++++++++++++
>>   drivers/media/platform/soc_camera/atmel-isi.c      |   33 ++++++++++++-
>>   2 files changed, 82 insertions(+), 2 deletions(-)
>>   create mode 100644 Documentation/devicetree/bindings/media/atmel-isi.txt
>>
>> diff --git a/Documentation/devicetree/bindings/media/atmel-isi.txt
>> b/Documentation/devicetree/bindings/media/atmel-isi.txt new file mode
>> 100644
>> index 0000000..07f00eb
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/atmel-isi.txt
>> @@ -0,0 +1,51 @@
>> +Atmel Image Sensor Interface (ISI) SoC Camera Subsystem
>> +----------------------------------------------
>> +
>> +Required properties:
>> +- compatible: must be "atmel,at91sam9g45-isi"
>> +- reg: physical base address and length of the registers set for the
>> device;
>> +- interrupts: should contain IRQ line for the ISI;
>> +- clocks: list of clock specifiers, corresponding to entries in
>> +          the clock-names property;
>> +- clock-names: must contain "isi_clk", which is the isi peripherial clock.
>> +               "isi_mck" is optinal, it is the master clock output to
>> sensor.
> The mck clock should be handled by the sensor driver instead. I know we have a
> legacy mode in the atmel-isi driver to manage that clock internally, but let's
> not propagate that to DT.

I agree with you.

I put the isi_mck as optional here because current the sensor driver 
code only managed the v4l2 clock not the common clock.
There should add additional code to manager mck clock.
So if you want to ISI work for now, you should put the isi_mck in 
atmel-isi DT node.

But for sure I can remove the isi_mck in atmel-isi DT document. In the 
future it will be add in sensor's DT document.

> I would also drop the "isi_" prefix from the isi_clk
> name.

hmm,  I think "isi_clk" indicates it is a ISI peripheral clock. And 
which is consistent with other peripheral clock name in sama5.

>
> You should also describe the port node. You can just mention the related
> bindings document, and state that the ISI has a single port.

OK. will add in the v2.

>
>> +Optional properties:
>> +- atmel,isi-disable-preview: a boolean property to disable the preview
>> channel;
> That doesn't really sound like a hardware property to me. Isn't it full mode
> related to software configuration instead, which should be performed at
> runtime by userspace ?

yes, this configuration can be disable/enable by driver according to 
user select format.
I will remove it in v2. Thanks.

Best Regards,
Josh Wu

>
>> +
>> +Example:
>> +	isi: isi at f0034000 {
>> +		compatible = "atmel,at91sam9g45-isi";
>> +		reg = <0xf0034000 0x4000>;
>> +		interrupts = <37 IRQ_TYPE_LEVEL_HIGH 5>;
>> +
>> +		clocks = <&isi_clk>, <&pck1>;
>> +		clock-names = "isi_clk", "isi_mck";
>> +
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&pinctrl_isi &pinctrl_pck1_as_isi_mck>;
>> +
>> +		port {
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +
>> +			isi_0: endpoint {
>> +				remote-endpoint = <&ov2640_0>;
>> +			};
>> +		};
>> +	};
>> +
>> +	i2c1: i2c at f0018000 {
>> +		ov2640: camera at 0x30 {
>> +			compatible = "omnivision,ov2640";
>> +			reg = <0x30>;
>> +
>> +			port {
>> +				ov2640_0: endpoint {
>> +					remote-endpoint = <&isi_0>;
>> +					bus-width = <8>;
>> +				};
>> +			};
>> +		};
>> +	};
>> +
>> diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
>> b/drivers/media/platform/soc_camera/atmel-isi.c index 93bf1cb..1822129
>> 100644
>> --- a/drivers/media/platform/soc_camera/atmel-isi.c
>> +++ b/drivers/media/platform/soc_camera/atmel-isi.c
>> @@ -19,6 +19,7 @@
>>   #include <linux/interrupt.h>
>>   #include <linux/kernel.h>
>>   #include <linux/module.h>
>> +#include <linux/of.h>
>>   #include <linux/platform_device.h>
>>   #include <linux/slab.h>
>>
>> @@ -33,6 +34,7 @@
>>   #define VID_LIMIT_BYTES			(16 * 1024 * 1024)
>>   #define MIN_FRAME_RATE			15
>>   #define FRAME_INTERVAL_MILLI_SEC	(1000 / MIN_FRAME_RATE)
>> +#define ISI_DEFAULT_MCLK_FREQ		25000000
>>
>>   /* Frame buffer descriptor */
>>   struct fbd {
>> @@ -878,6 +880,22 @@ static int atmel_isi_remove(struct platform_device
>> *pdev) return 0;
>>   }
>>
>> +static int atmel_isi_probe_dt(struct atmel_isi *isi,
>> +			struct platform_device *pdev)
>> +{
>> +	struct device_node *node = pdev->dev.of_node;
>> +
>> +	isi->pdata.full_mode = !of_property_read_bool(node,
>> +			"atmel,isi-disable-preview");
>> +
>> +	/* Default settings for ISI */
>> +	isi->pdata.mck_hz = ISI_DEFAULT_MCLK_FREQ;
>> +	isi->pdata.frate = ISI_CFG1_FRATE_CAPTURE_ALL;
>> +	isi->pdata.data_width_flags = ISI_DATAWIDTH_8 | ISI_DATAWIDTH_10;
>> +
>> +	return 0;
>> +}
>> +
>>   static int atmel_isi_probe(struct platform_device *pdev)
>>   {
>>   	unsigned int irq;
>> @@ -889,7 +907,7 @@ static int atmel_isi_probe(struct platform_device *pdev)
>> struct isi_platform_data *pdata;
>>
>>   	pdata = dev->platform_data;
>> -	if (!pdata || !pdata->data_width_flags) {
>> +	if ((!pdata || !pdata->data_width_flags) && !pdev->dev.of_node) {
>>   		dev_err(&pdev->dev,
>>   			"No config available for Atmel ISI\n");
>>   		return -EINVAL;
>> @@ -905,7 +923,11 @@ static int atmel_isi_probe(struct platform_device
>> *pdev) if (IS_ERR(isi->pclk))
>>   		return PTR_ERR(isi->pclk);
>>
>> -	memcpy(&isi->pdata, pdata, sizeof(struct isi_platform_data));
>> +	if (pdata)
>> +		memcpy(&isi->pdata, pdata, sizeof(struct isi_platform_data));
>> +	else	/* dt probe */
>> +		atmel_isi_probe_dt(isi, pdev);
>> +
>>   	isi->active = NULL;
>>   	spin_lock_init(&isi->lock);
>>   	INIT_LIST_HEAD(&isi->video_buffer_list);
>> @@ -1007,11 +1029,18 @@ err_alloc_ctx:
>>   	return ret;
>>   }
>>
>> +static const struct of_device_id atmel_isi_of_match[] = {
>> +	{ .compatible = "atmel,at91sam9g45-isi" },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(of, atmel_isi_of_match);
>> +
>>   static struct platform_driver atmel_isi_driver = {
>>   	.remove		= atmel_isi_remove,
>>   	.driver		= {
>>   		.name = "atmel_isi",
>>   		.owner = THIS_MODULE,
>> +		.of_match_table = atmel_isi_of_match,
>>   	},
>>   };

Re: [PATCH 3/3] [media] atmel-isi: add primary DT support

[Josh Wu]

Hi, Laurent

On 3/18/2014 9:36 PM, Laurent Pinchart wrote:
> Hi Josh,
>
> Thank you for the patch.

Thanks for your review.

>
> On Tuesday 18 March 2014 19:19:54 Josh Wu wrote:
>> This patch add the DT support for Atmel ISI driver.
>> It use the same v4l2 DT interface that defined in video-interfaces.txt.
>>
>> Signed-off-by: Josh Wu <josh.wu@atmel.com>
>> Cc: devicetree@vger.kernel.org
>> ---
>>   .../devicetree/bindings/media/atmel-isi.txt        |   51 +++++++++++++++++
>>   drivers/media/platform/soc_camera/atmel-isi.c      |   33 ++++++++++++-
>>   2 files changed, 82 insertions(+), 2 deletions(-)
>>   create mode 100644 Documentation/devicetree/bindings/media/atmel-isi.txt
>>
>> diff --git a/Documentation/devicetree/bindings/media/atmel-isi.txt
>> b/Documentation/devicetree/bindings/media/atmel-isi.txt new file mode
>> 100644
>> index 0000000..07f00eb
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/atmel-isi.txt
>> @@ -0,0 +1,51 @@
>> +Atmel Image Sensor Interface (ISI) SoC Camera Subsystem
>> +----------------------------------------------
>> +
>> +Required properties:
>> +- compatible: must be "atmel,at91sam9g45-isi"
>> +- reg: physical base address and length of the registers set for the
>> device;
>> +- interrupts: should contain IRQ line for the ISI;
>> +- clocks: list of clock specifiers, corresponding to entries in
>> +          the clock-names property;
>> +- clock-names: must contain "isi_clk", which is the isi peripherial clock.
>> +               "isi_mck" is optinal, it is the master clock output to
>> sensor.
> The mck clock should be handled by the sensor driver instead. I know we have a
> legacy mode in the atmel-isi driver to manage that clock internally, but let's
> not propagate that to DT.

I agree with you.

I put the isi_mck as optional here because current the sensor driver 
code only managed the v4l2 clock not the common clock.
There should add additional code to manager mck clock.
So if you want to ISI work for now, you should put the isi_mck in 
atmel-isi DT node.

But for sure I can remove the isi_mck in atmel-isi DT document. In the 
future it will be add in sensor's DT document.

> I would also drop the "isi_" prefix from the isi_clk
> name.

hmm,  I think "isi_clk" indicates it is a ISI peripheral clock. And 
which is consistent with other peripheral clock name in sama5.

>
> You should also describe the port node. You can just mention the related
> bindings document, and state that the ISI has a single port.

OK. will add in the v2.

>
>> +Optional properties:
>> +- atmel,isi-disable-preview: a boolean property to disable the preview
>> channel;
> That doesn't really sound like a hardware property to me. Isn't it full mode
> related to software configuration instead, which should be performed at
> runtime by userspace ?

yes, this configuration can be disable/enable by driver according to 
user select format.
I will remove it in v2. Thanks.

Best Regards,
Josh Wu

>
>> +
>> +Example:
>> +	isi: isi@f0034000 {
>> +		compatible = "atmel,at91sam9g45-isi";
>> +		reg = <0xf0034000 0x4000>;
>> +		interrupts = <37 IRQ_TYPE_LEVEL_HIGH 5>;
>> +
>> +		clocks = <&isi_clk>, <&pck1>;
>> +		clock-names = "isi_clk", "isi_mck";
>> +
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&pinctrl_isi &pinctrl_pck1_as_isi_mck>;
>> +
>> +		port {
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +
>> +			isi_0: endpoint {
>> +				remote-endpoint = <&ov2640_0>;
>> +			};
>> +		};
>> +	};
>> +
>> +	i2c1: i2c@f0018000 {
>> +		ov2640: camera@0x30 {
>> +			compatible = "omnivision,ov2640";
>> +			reg = <0x30>;
>> +
>> +			port {
>> +				ov2640_0: endpoint {
>> +					remote-endpoint = <&isi_0>;
>> +					bus-width = <8>;
>> +				};
>> +			};
>> +		};
>> +	};
>> +
>> diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
>> b/drivers/media/platform/soc_camera/atmel-isi.c index 93bf1cb..1822129
>> 100644
>> --- a/drivers/media/platform/soc_camera/atmel-isi.c
>> +++ b/drivers/media/platform/soc_camera/atmel-isi.c
>> @@ -19,6 +19,7 @@
>>   #include <linux/interrupt.h>
>>   #include <linux/kernel.h>
>>   #include <linux/module.h>
>> +#include <linux/of.h>
>>   #include <linux/platform_device.h>
>>   #include <linux/slab.h>
>>
>> @@ -33,6 +34,7 @@
>>   #define VID_LIMIT_BYTES			(16 * 1024 * 1024)
>>   #define MIN_FRAME_RATE			15
>>   #define FRAME_INTERVAL_MILLI_SEC	(1000 / MIN_FRAME_RATE)
>> +#define ISI_DEFAULT_MCLK_FREQ		25000000
>>
>>   /* Frame buffer descriptor */
>>   struct fbd {
>> @@ -878,6 +880,22 @@ static int atmel_isi_remove(struct platform_device
>> *pdev) return 0;
>>   }
>>
>> +static int atmel_isi_probe_dt(struct atmel_isi *isi,
>> +			struct platform_device *pdev)
>> +{
>> +	struct device_node *node = pdev->dev.of_node;
>> +
>> +	isi->pdata.full_mode = !of_property_read_bool(node,
>> +			"atmel,isi-disable-preview");
>> +
>> +	/* Default settings for ISI */
>> +	isi->pdata.mck_hz = ISI_DEFAULT_MCLK_FREQ;
>> +	isi->pdata.frate = ISI_CFG1_FRATE_CAPTURE_ALL;
>> +	isi->pdata.data_width_flags = ISI_DATAWIDTH_8 | ISI_DATAWIDTH_10;
>> +
>> +	return 0;
>> +}
>> +
>>   static int atmel_isi_probe(struct platform_device *pdev)
>>   {
>>   	unsigned int irq;
>> @@ -889,7 +907,7 @@ static int atmel_isi_probe(struct platform_device *pdev)
>> struct isi_platform_data *pdata;
>>
>>   	pdata = dev->platform_data;
>> -	if (!pdata || !pdata->data_width_flags) {
>> +	if ((!pdata || !pdata->data_width_flags) && !pdev->dev.of_node) {
>>   		dev_err(&pdev->dev,
>>   			"No config available for Atmel ISI\n");
>>   		return -EINVAL;
>> @@ -905,7 +923,11 @@ static int atmel_isi_probe(struct platform_device
>> *pdev) if (IS_ERR(isi->pclk))
>>   		return PTR_ERR(isi->pclk);
>>
>> -	memcpy(&isi->pdata, pdata, sizeof(struct isi_platform_data));
>> +	if (pdata)
>> +		memcpy(&isi->pdata, pdata, sizeof(struct isi_platform_data));
>> +	else	/* dt probe */
>> +		atmel_isi_probe_dt(isi, pdev);
>> +
>>   	isi->active = NULL;
>>   	spin_lock_init(&isi->lock);
>>   	INIT_LIST_HEAD(&isi->video_buffer_list);
>> @@ -1007,11 +1029,18 @@ err_alloc_ctx:
>>   	return ret;
>>   }
>>
>> +static const struct of_device_id atmel_isi_of_match[] = {
>> +	{ .compatible = "atmel,at91sam9g45-isi" },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(of, atmel_isi_of_match);
>> +
>>   static struct platform_driver atmel_isi_driver = {
>>   	.remove		= atmel_isi_remove,
>>   	.driver		= {
>>   		.name = "atmel_isi",
>>   		.owner = THIS_MODULE,
>> +		.of_match_table = atmel_isi_of_match,
>>   	},
>>   };

Re: [PATCH 3/3] [media] atmel-isi: add primary DT support

[Josh Wu]

Hi, Laurent

On 3/18/2014 9:36 PM, Laurent Pinchart wrote:
> Hi Josh,
>
> Thank you for the patch.

Thanks for your review.

>
> On Tuesday 18 March 2014 19:19:54 Josh Wu wrote:
>> This patch add the DT support for Atmel ISI driver.
>> It use the same v4l2 DT interface that defined in video-interfaces.txt.
>>
>> Signed-off-by: Josh Wu <josh.wu@atmel.com>
>> Cc: devicetree@vger.kernel.org
>> ---
>>   .../devicetree/bindings/media/atmel-isi.txt        |   51 +++++++++++++++++
>>   drivers/media/platform/soc_camera/atmel-isi.c      |   33 ++++++++++++-
>>   2 files changed, 82 insertions(+), 2 deletions(-)
>>   create mode 100644 Documentation/devicetree/bindings/media/atmel-isi.txt
>>
>> diff --git a/Documentation/devicetree/bindings/media/atmel-isi.txt
>> b/Documentation/devicetree/bindings/media/atmel-isi.txt new file mode
>> 100644
>> index 0000000..07f00eb
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/atmel-isi.txt
>> @@ -0,0 +1,51 @@
>> +Atmel Image Sensor Interface (ISI) SoC Camera Subsystem
>> +----------------------------------------------
>> +
>> +Required properties:
>> +- compatible: must be "atmel,at91sam9g45-isi"
>> +- reg: physical base address and length of the registers set for the
>> device;
>> +- interrupts: should contain IRQ line for the ISI;
>> +- clocks: list of clock specifiers, corresponding to entries in
>> +          the clock-names property;
>> +- clock-names: must contain "isi_clk", which is the isi peripherial clock.
>> +               "isi_mck" is optinal, it is the master clock output to
>> sensor.
> The mck clock should be handled by the sensor driver instead. I know we have a
> legacy mode in the atmel-isi driver to manage that clock internally, but let's
> not propagate that to DT.

I agree with you.

I put the isi_mck as optional here because current the sensor driver 
code only managed the v4l2 clock not the common clock.
There should add additional code to manager mck clock.
So if you want to ISI work for now, you should put the isi_mck in 
atmel-isi DT node.

But for sure I can remove the isi_mck in atmel-isi DT document. In the 
future it will be add in sensor's DT document.

> I would also drop the "isi_" prefix from the isi_clk
> name.

hmm,  I think "isi_clk" indicates it is a ISI peripheral clock. And 
which is consistent with other peripheral clock name in sama5.

>
> You should also describe the port node. You can just mention the related
> bindings document, and state that the ISI has a single port.

OK. will add in the v2.

>
>> +Optional properties:
>> +- atmel,isi-disable-preview: a boolean property to disable the preview
>> channel;
> That doesn't really sound like a hardware property to me. Isn't it full mode
> related to software configuration instead, which should be performed at
> runtime by userspace ?

yes, this configuration can be disable/enable by driver according to 
user select format.
I will remove it in v2. Thanks.

Best Regards,
Josh Wu

>
>> +
>> +Example:
>> +	isi: isi@f0034000 {
>> +		compatible = "atmel,at91sam9g45-isi";
>> +		reg = <0xf0034000 0x4000>;
>> +		interrupts = <37 IRQ_TYPE_LEVEL_HIGH 5>;
>> +
>> +		clocks = <&isi_clk>, <&pck1>;
>> +		clock-names = "isi_clk", "isi_mck";
>> +
>> +		pinctrl-names = "default";
>> +		pinctrl-0 = <&pinctrl_isi &pinctrl_pck1_as_isi_mck>;
>> +
>> +		port {
>> +			#address-cells = <1>;
>> +			#size-cells = <0>;
>> +
>> +			isi_0: endpoint {
>> +				remote-endpoint = <&ov2640_0>;
>> +			};
>> +		};
>> +	};
>> +
>> +	i2c1: i2c@f0018000 {
>> +		ov2640: camera@0x30 {
>> +			compatible = "omnivision,ov2640";
>> +			reg = <0x30>;
>> +
>> +			port {
>> +				ov2640_0: endpoint {
>> +					remote-endpoint = <&isi_0>;
>> +					bus-width = <8>;
>> +				};
>> +			};
>> +		};
>> +	};
>> +
>> diff --git a/drivers/media/platform/soc_camera/atmel-isi.c
>> b/drivers/media/platform/soc_camera/atmel-isi.c index 93bf1cb..1822129
>> 100644
>> --- a/drivers/media/platform/soc_camera/atmel-isi.c
>> +++ b/drivers/media/platform/soc_camera/atmel-isi.c
>> @@ -19,6 +19,7 @@
>>   #include <linux/interrupt.h>
>>   #include <linux/kernel.h>
>>   #include <linux/module.h>
>> +#include <linux/of.h>
>>   #include <linux/platform_device.h>
>>   #include <linux/slab.h>
>>
>> @@ -33,6 +34,7 @@
>>   #define VID_LIMIT_BYTES			(16 * 1024 * 1024)
>>   #define MIN_FRAME_RATE			15
>>   #define FRAME_INTERVAL_MILLI_SEC	(1000 / MIN_FRAME_RATE)
>> +#define ISI_DEFAULT_MCLK_FREQ		25000000
>>
>>   /* Frame buffer descriptor */
>>   struct fbd {
>> @@ -878,6 +880,22 @@ static int atmel_isi_remove(struct platform_device
>> *pdev) return 0;
>>   }
>>
>> +static int atmel_isi_probe_dt(struct atmel_isi *isi,
>> +			struct platform_device *pdev)
>> +{
>> +	struct device_node *node = pdev->dev.of_node;
>> +
>> +	isi->pdata.full_mode = !of_property_read_bool(node,
>> +			"atmel,isi-disable-preview");
>> +
>> +	/* Default settings for ISI */
>> +	isi->pdata.mck_hz = ISI_DEFAULT_MCLK_FREQ;
>> +	isi->pdata.frate = ISI_CFG1_FRATE_CAPTURE_ALL;
>> +	isi->pdata.data_width_flags = ISI_DATAWIDTH_8 | ISI_DATAWIDTH_10;
>> +
>> +	return 0;
>> +}
>> +
>>   static int atmel_isi_probe(struct platform_device *pdev)
>>   {
>>   	unsigned int irq;
>> @@ -889,7 +907,7 @@ static int atmel_isi_probe(struct platform_device *pdev)
>> struct isi_platform_data *pdata;
>>
>>   	pdata = dev->platform_data;
>> -	if (!pdata || !pdata->data_width_flags) {
>> +	if ((!pdata || !pdata->data_width_flags) && !pdev->dev.of_node) {
>>   		dev_err(&pdev->dev,
>>   			"No config available for Atmel ISI\n");
>>   		return -EINVAL;
>> @@ -905,7 +923,11 @@ static int atmel_isi_probe(struct platform_device
>> *pdev) if (IS_ERR(isi->pclk))
>>   		return PTR_ERR(isi->pclk);
>>
>> -	memcpy(&isi->pdata, pdata, sizeof(struct isi_platform_data));
>> +	if (pdata)
>> +		memcpy(&isi->pdata, pdata, sizeof(struct isi_platform_data));
>> +	else	/* dt probe */
>> +		atmel_isi_probe_dt(isi, pdev);
>> +
>>   	isi->active = NULL;
>>   	spin_lock_init(&isi->lock);
>>   	INIT_LIST_HEAD(&isi->video_buffer_list);
>> @@ -1007,11 +1029,18 @@ err_alloc_ctx:
>>   	return ret;
>>   }
>>
>> +static const struct of_device_id atmel_isi_of_match[] = {
>> +	{ .compatible = "atmel,at91sam9g45-isi" },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(of, atmel_isi_of_match);
>> +
>>   static struct platform_driver atmel_isi_driver = {
>>   	.remove		= atmel_isi_remove,
>>   	.driver		= {
>>   		.name = "atmel_isi",
>>   		.owner = THIS_MODULE,
>> +		.of_match_table = atmel_isi_of_match,
>>   	},
>>   };

[PATCH 3/3] [media] atmel-isi: add primary DT support

[Laurent Pinchart]

Hi Josh,

On Wednesday 19 March 2014 17:15:02 Josh Wu wrote:
> On 3/18/2014 9:36 PM, Laurent Pinchart wrote:
> > On Tuesday 18 March 2014 19:19:54 Josh Wu wrote:
> >> This patch add the DT support for Atmel ISI driver.
> >> It use the same v4l2 DT interface that defined in video-interfaces.txt.
> >> 
> >> Signed-off-by: Josh Wu <josh.wu@atmel.com>
> >> Cc: devicetree at vger.kernel.org
> >> ---
> >> 
> >>  .../devicetree/bindings/media/atmel-isi.txt        |   51 ++++++++++++++
> >>  drivers/media/platform/soc_camera/atmel-isi.c      |   33 ++++++++++++-
> >>  2 files changed, 82 insertions(+), 2 deletions(-)
> >>  create mode 100644
> >>  Documentation/devicetree/bindings/media/atmel-isi.txt
> >> 
> >> diff --git a/Documentation/devicetree/bindings/media/atmel-isi.txt
> >> b/Documentation/devicetree/bindings/media/atmel-isi.txt new file mode
> >> 100644
> >> index 0000000..07f00eb
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/media/atmel-isi.txt
> >> @@ -0,0 +1,51 @@
> >> +Atmel Image Sensor Interface (ISI) SoC Camera Subsystem
> >> +----------------------------------------------
> >> +
> >> +Required properties:
> >> +- compatible: must be "atmel,at91sam9g45-isi"
> >> +- reg: physical base address and length of the registers set for the
> >> device;
> >> +- interrupts: should contain IRQ line for the ISI;
> >> +- clocks: list of clock specifiers, corresponding to entries in
> >> +          the clock-names property;
> >> +- clock-names: must contain "isi_clk", which is the isi peripherial
> >> clock.
> >> +               "isi_mck" is optinal, it is the master clock output to
> >> sensor.
> > 
> > The mck clock should be handled by the sensor driver instead. I know we
> > have a legacy mode in the atmel-isi driver to manage that clock
> > internally, but let's not propagate that to DT.
> 
> I agree with you.
> 
> I put the isi_mck as optional here because current the sensor driver
> code only managed the v4l2 clock not the common clock.
> There should add additional code to manager mck clock.
> So if you want to ISI work for now, you should put the isi_mck in
> atmel-isi DT node.
> 
> But for sure I can remove the isi_mck in atmel-isi DT document. In the
> future it will be add in sensor's DT document.

I think that's the way to go, yes. I know we have existing platforms that 
require sensor clock management in the ISI driver. That should be fixed, and a 
move to DT is a perfect opportunity to do so :-)

> > I would also drop the "isi_" prefix from the isi_clk name.
> 
> hmm,  I think "isi_clk" indicates it is a ISI peripheral clock. And
> which is consistent with other peripheral clock name in sama5.

I believe the "isi_" prefix is redundant, given that the clock-names property 
is inside the ISI DT node. However, if this style matches the rest of the 
platform there's no need to change it.

> > You should also describe the port node. You can just mention the related
> > bindings document, and state that the ISI has a single port.
> 
> OK. will add in the v2.
> 
> >> +Optional properties:
> >> +- atmel,isi-disable-preview: a boolean property to disable the preview
> >> channel;
> > 
> > That doesn't really sound like a hardware property to me. Isn't it full
> > mode related to software configuration instead, which should be performed
> > at runtime by userspace ?
> 
> yes, this configuration can be disable/enable by driver according to
> user select format.
> I will remove it in v2. Thanks.
> 
> Best Regards,
> Josh Wu
> 
> >> +
> >> +Example:
> >> +	isi: isi at f0034000 {
> >> +		compatible = "atmel,at91sam9g45-isi";
> >> +		reg = <0xf0034000 0x4000>;
> >> +		interrupts = <37 IRQ_TYPE_LEVEL_HIGH 5>;
> >> +
> >> +		clocks = <&isi_clk>, <&pck1>;
> >> +		clock-names = "isi_clk", "isi_mck";
> >> +
> >> +		pinctrl-names = "default";
> >> +		pinctrl-0 = <&pinctrl_isi &pinctrl_pck1_as_isi_mck>;
> >> +
> >> +		port {
> >> +			#address-cells = <1>;
> >> +			#size-cells = <0>;
> >> +
> >> +			isi_0: endpoint {
> >> +				remote-endpoint = <&ov2640_0>;
> >> +			};
> >> +		};
> >> +	};
> >> +
> >> +	i2c1: i2c at f0018000 {
> >> +		ov2640: camera at 0x30 {
> >> +			compatible = "omnivision,ov2640";
> >> +			reg = <0x30>;
> >> +
> >> +			port {
> >> +				ov2640_0: endpoint {
> >> +					remote-endpoint = <&isi_0>;
> >> +					bus-width = <8>;
> >> +				};
> >> +			};
> >> +		};
> >> +	};

-- 
Regards,

Laurent Pinchart

Re: [PATCH 3/3] [media] atmel-isi: add primary DT support

[Laurent Pinchart]

Hi Josh,

On Wednesday 19 March 2014 17:15:02 Josh Wu wrote:
> On 3/18/2014 9:36 PM, Laurent Pinchart wrote:
> > On Tuesday 18 March 2014 19:19:54 Josh Wu wrote:
> >> This patch add the DT support for Atmel ISI driver.
> >> It use the same v4l2 DT interface that defined in video-interfaces.txt.
> >> 
> >> Signed-off-by: Josh Wu <josh.wu@atmel.com>
> >> Cc: devicetree@vger.kernel.org
> >> ---
> >> 
> >>  .../devicetree/bindings/media/atmel-isi.txt        |   51 ++++++++++++++
> >>  drivers/media/platform/soc_camera/atmel-isi.c      |   33 ++++++++++++-
> >>  2 files changed, 82 insertions(+), 2 deletions(-)
> >>  create mode 100644
> >>  Documentation/devicetree/bindings/media/atmel-isi.txt
> >> 
> >> diff --git a/Documentation/devicetree/bindings/media/atmel-isi.txt
> >> b/Documentation/devicetree/bindings/media/atmel-isi.txt new file mode
> >> 100644
> >> index 0000000..07f00eb
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/media/atmel-isi.txt
> >> @@ -0,0 +1,51 @@
> >> +Atmel Image Sensor Interface (ISI) SoC Camera Subsystem
> >> +----------------------------------------------
> >> +
> >> +Required properties:
> >> +- compatible: must be "atmel,at91sam9g45-isi"
> >> +- reg: physical base address and length of the registers set for the
> >> device;
> >> +- interrupts: should contain IRQ line for the ISI;
> >> +- clocks: list of clock specifiers, corresponding to entries in
> >> +          the clock-names property;
> >> +- clock-names: must contain "isi_clk", which is the isi peripherial
> >> clock.
> >> +               "isi_mck" is optinal, it is the master clock output to
> >> sensor.
> > 
> > The mck clock should be handled by the sensor driver instead. I know we
> > have a legacy mode in the atmel-isi driver to manage that clock
> > internally, but let's not propagate that to DT.
> 
> I agree with you.
> 
> I put the isi_mck as optional here because current the sensor driver
> code only managed the v4l2 clock not the common clock.
> There should add additional code to manager mck clock.
> So if you want to ISI work for now, you should put the isi_mck in
> atmel-isi DT node.
> 
> But for sure I can remove the isi_mck in atmel-isi DT document. In the
> future it will be add in sensor's DT document.

I think that's the way to go, yes. I know we have existing platforms that 
require sensor clock management in the ISI driver. That should be fixed, and a 
move to DT is a perfect opportunity to do so :-)

> > I would also drop the "isi_" prefix from the isi_clk name.
> 
> hmm,  I think "isi_clk" indicates it is a ISI peripheral clock. And
> which is consistent with other peripheral clock name in sama5.

I believe the "isi_" prefix is redundant, given that the clock-names property 
is inside the ISI DT node. However, if this style matches the rest of the 
platform there's no need to change it.

> > You should also describe the port node. You can just mention the related
> > bindings document, and state that the ISI has a single port.
> 
> OK. will add in the v2.
> 
> >> +Optional properties:
> >> +- atmel,isi-disable-preview: a boolean property to disable the preview
> >> channel;
> > 
> > That doesn't really sound like a hardware property to me. Isn't it full
> > mode related to software configuration instead, which should be performed
> > at runtime by userspace ?
> 
> yes, this configuration can be disable/enable by driver according to
> user select format.
> I will remove it in v2. Thanks.
> 
> Best Regards,
> Josh Wu
> 
> >> +
> >> +Example:
> >> +	isi: isi@f0034000 {
> >> +		compatible = "atmel,at91sam9g45-isi";
> >> +		reg = <0xf0034000 0x4000>;
> >> +		interrupts = <37 IRQ_TYPE_LEVEL_HIGH 5>;
> >> +
> >> +		clocks = <&isi_clk>, <&pck1>;
> >> +		clock-names = "isi_clk", "isi_mck";
> >> +
> >> +		pinctrl-names = "default";
> >> +		pinctrl-0 = <&pinctrl_isi &pinctrl_pck1_as_isi_mck>;
> >> +
> >> +		port {
> >> +			#address-cells = <1>;
> >> +			#size-cells = <0>;
> >> +
> >> +			isi_0: endpoint {
> >> +				remote-endpoint = <&ov2640_0>;
> >> +			};
> >> +		};
> >> +	};
> >> +
> >> +	i2c1: i2c@f0018000 {
> >> +		ov2640: camera@0x30 {
> >> +			compatible = "omnivision,ov2640";
> >> +			reg = <0x30>;
> >> +
> >> +			port {
> >> +				ov2640_0: endpoint {
> >> +					remote-endpoint = <&isi_0>;
> >> +					bus-width = <8>;
> >> +				};
> >> +			};
> >> +		};
> >> +	};

-- 
Regards,

Laurent Pinchart