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

2014-07-26 Thread Guennadi Liakhovetski
Hi Josh,

Thanks for a prompt update! A couple of minor questions:

On Fri, 25 Jul 2014, 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...@atmel.com
 Cc: devicet...@vger.kernel.org
 Reviewed-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
 ---
 v2 - v3:
   add bus-width property support.
   add error handling when calling atmel_isi_probe_dt().
 
 v1 - v2:
   refine the binding document.
   add port node description.
   removed the optional property.
 
  .../devicetree/bindings/media/atmel-isi.txt| 51 +
  drivers/media/platform/soc_camera/atmel-isi.c  | 64 
 +-
  2 files changed, 113 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 000..17e71b7
 --- /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 supports a single port node with parallel bus. It should contain one
 +'port' child node with child 'endpoint' node. Please refer to the bindings
 +defined in Documentation/devicetree/bindings/media/video-interfaces.txt.
 +
 +Example:
 + isi: isi@f0034000 {
 + compatible = atmel,at91sam9g45-isi;
 + reg = 0xf0034000 0x4000;
 + interrupts = 37 IRQ_TYPE_LEVEL_HIGH 5;
 +
 + clocks = isi_clk;
 + clock-names = isi_clk;
 +
 + pinctrl-names = default;
 + pinctrl-0 = pinctrl_isi;
 +
 + port {
 + #address-cells = 1;
 + #size-cells = 0;
 +
 + isi_0: endpoint {
 + remote-endpoint = ov2640_0;
 + bus-width = 8;
 + };
 + };
 + };
 +
 + 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 74af560..ca4e43e 100644
 --- a/drivers/media/platform/soc_camera/atmel-isi.c
 +++ b/drivers/media/platform/soc_camera/atmel-isi.c
 @@ -25,6 +25,7 @@
  #include media/atmel-isi.h
  #include media/soc_camera.h
  #include media/soc_mediabus.h
 +#include media/v4l2-of.h
  #include media/videobuf2-dma-contig.h
  
  #define MAX_BUFFER_NUM   32
 @@ -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_FREQ2500
  
  /* Frame buffer descriptor */
  struct fbd {
 @@ -883,6 +885,50 @@ 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 *np= pdev-dev.of_node;
 + struct v4l2_of_endpoint ep;
 + int err;
 +
 + /* Default settings for ISI */
 + isi-pdata.full_mode = 1;
 + isi-pdata.mck_hz = ISI_DEFAULT_MCLK_FREQ;
 + isi-pdata.frate = ISI_CFG1_FRATE_CAPTURE_ALL;
 +
 + np = of_graph_get_next_endpoint(np, NULL);
 + if (!np) {
 + dev_err(pdev-dev, Could not find the endpoint\n);
 + return -EINVAL;
 + }
 +
 + err = v4l2_of_parse_endpoint(np, ep);
 + if (err) {
 + dev_err(pdev-dev, Could not parse the endpoint\n);
 + goto err_probe_dt;
 + }
 +
 + switch (ep.bus.parallel.bus_width) {
 + case 8:
 + isi-pdata.data_width_flags = ISI_DATAWIDTH_8;
 + break;
 + case 10:
 + isi-pdata.data_width_flags = ISI_DATAWIDTH_10;
 + break;

Wouldn't it be natural, if bus_width=10 is specified in DT to enable both 
ISI_DATAWIDTH_8 and ISI_DATAWIDTH_10?

 + default:
 + dev_err(pdev-dev, 

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

2014-07-25 Thread 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...@atmel.com
Cc: devicet...@vger.kernel.org
Reviewed-by: Laurent Pinchart laurent.pinch...@ideasonboard.com
---
v2 - v3:
  add bus-width property support.
  add error handling when calling atmel_isi_probe_dt().

v1 - v2:
  refine the binding document.
  add port node description.
  removed the optional property.

 .../devicetree/bindings/media/atmel-isi.txt| 51 +
 drivers/media/platform/soc_camera/atmel-isi.c  | 64 +-
 2 files changed, 113 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 000..17e71b7
--- /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 supports a single port node with parallel bus. It should contain one
+'port' child node with child 'endpoint' node. Please refer to the bindings
+defined in Documentation/devicetree/bindings/media/video-interfaces.txt.
+
+Example:
+   isi: isi@f0034000 {
+   compatible = atmel,at91sam9g45-isi;
+   reg = 0xf0034000 0x4000;
+   interrupts = 37 IRQ_TYPE_LEVEL_HIGH 5;
+
+   clocks = isi_clk;
+   clock-names = isi_clk;
+
+   pinctrl-names = default;
+   pinctrl-0 = pinctrl_isi;
+
+   port {
+   #address-cells = 1;
+   #size-cells = 0;
+
+   isi_0: endpoint {
+   remote-endpoint = ov2640_0;
+   bus-width = 8;
+   };
+   };
+   };
+
+   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 74af560..ca4e43e 100644
--- a/drivers/media/platform/soc_camera/atmel-isi.c
+++ b/drivers/media/platform/soc_camera/atmel-isi.c
@@ -25,6 +25,7 @@
 #include media/atmel-isi.h
 #include media/soc_camera.h
 #include media/soc_mediabus.h
+#include media/v4l2-of.h
 #include media/videobuf2-dma-contig.h
 
 #define MAX_BUFFER_NUM 32
@@ -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  2500
 
 /* Frame buffer descriptor */
 struct fbd {
@@ -883,6 +885,50 @@ 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 *np= pdev-dev.of_node;
+   struct v4l2_of_endpoint ep;
+   int err;
+
+   /* Default settings for ISI */
+   isi-pdata.full_mode = 1;
+   isi-pdata.mck_hz = ISI_DEFAULT_MCLK_FREQ;
+   isi-pdata.frate = ISI_CFG1_FRATE_CAPTURE_ALL;
+
+   np = of_graph_get_next_endpoint(np, NULL);
+   if (!np) {
+   dev_err(pdev-dev, Could not find the endpoint\n);
+   return -EINVAL;
+   }
+
+   err = v4l2_of_parse_endpoint(np, ep);
+   if (err) {
+   dev_err(pdev-dev, Could not parse the endpoint\n);
+   goto err_probe_dt;
+   }
+
+   switch (ep.bus.parallel.bus_width) {
+   case 8:
+   isi-pdata.data_width_flags = ISI_DATAWIDTH_8;
+   break;
+   case 10:
+   isi-pdata.data_width_flags = ISI_DATAWIDTH_10;
+   break;
+   default:
+   dev_err(pdev-dev, Not supported bus width: %d\n,
+   ep.bus.parallel.bus_width);
+   err = -EINVAL;
+   goto err_probe_dt;
+   }
+
+err_probe_dt:
+   of_node_put(np);
+
+   return err;
+}
+
 static int