Re: [PATCH 6/7] ARM: at91: dts: sama5d3: add ov2640 camera sensor support

2014-12-22 Thread Alexandre Belloni
Hi Josh,

On 22/12/2014 at 15:06:07 +0800, Josh Wu wrote :
 I've acked your previous patch but maybe it should be named
 pinctrl_isi_pck1_as_mck to be clearer (you used the handle to pck1
 below).
 It's a good idea. Maybe I prefer to use the name: pinctrl_pck1_as_isi_mck ?
 If you are ok with this name, in next version, I will add one more patch in
 the series to do this.
 And I will keep your acked-by in my previous patch.
 

Sounds good to me!

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 6/7] ARM: at91: dts: sama5d3: add ov2640 camera sensor support

2014-12-21 Thread Josh Wu

Hi, Alexandre

Thanks for the review.

On 12/20/2014 5:05 AM, Alexandre Belloni wrote:

On 18/12/2014 at 16:51:06 +0800, Josh Wu wrote :

According to v4l2 dt document, we add:
   a camera host: ISI port.
   a i2c camera sensor: ov2640 port.
to sama5d3xmb.dtsi.

In the ov2640 node, it defines the pinctrls, clocks and isi port.
In the ISI node, it also reference to a ov2640 port.

Signed-off-by: Josh Wu josh...@atmel.com
---
  arch/arm/boot/dts/sama5d3xmb.dtsi | 32 
  1 file changed, 32 insertions(+)

diff --git a/arch/arm/boot/dts/sama5d3xmb.dtsi 
b/arch/arm/boot/dts/sama5d3xmb.dtsi
index 0aaebc6..958a528 100644
--- a/arch/arm/boot/dts/sama5d3xmb.dtsi
+++ b/arch/arm/boot/dts/sama5d3xmb.dtsi
@@ -52,6 +52,29 @@
};
};
  
+			i2c1: i2c@f0018000 {

+   ov2640: camera@0x30 {
+   compatible = ovti,ov2640;
+   reg = 0x30;
+   pinctrl-names = default;
+   pinctrl-0 = pinctrl_isi_pck_as_mck 
pinctrl_sensor_power pinctrl_sensor_reset;

I've acked your previous patch but maybe it should be named
pinctrl_isi_pck1_as_mck to be clearer (you used the handle to pck1
below).

It's a good idea. Maybe I prefer to use the name: pinctrl_pck1_as_isi_mck ?
If you are ok with this name, in next version, I will add one more patch 
in the series to do this.

And I will keep your acked-by in my previous patch.

Best Regards,
Josh Wu




+   resetb-gpios = pioE 24 
GPIO_ACTIVE_LOW;
+   pwdn-gpios = pioE 29 
GPIO_ACTIVE_HIGH;
+   /* use pck1 for the master clock of 
ov2640 */
+   clocks = pck1;
+   clock-names = xvclk;
+   assigned-clocks = pck1;
+   assigned-clock-rates = 2500;
+
+   port {
+   ov2640_0: endpoint {
+   remote-endpoint = 
isi_0;
+   bus-width = 8;
+   };
+   };
+   };
+   };
+
usart1: serial@f002 {
dmas = 0, 0;/*  Do not use DMA for 
usart1 */
pinctrl-names = default;
@@ -60,6 +83,15 @@
};
  
  			isi: isi@f0034000 {

+   port {
+   #address-cells = 1;
+   #size-cells = 0;
+
+   isi_0: endpoint {
+   remote-endpoint = ov2640_0;
+   bus-width = 8;
+   };
+   };
};
  
  			mmc1: mmc@f800 {

--
1.9.1



--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 6/7] ARM: at91: dts: sama5d3: add ov2640 camera sensor support

2014-12-19 Thread Alexandre Belloni
On 18/12/2014 at 16:51:06 +0800, Josh Wu wrote :
 According to v4l2 dt document, we add:
   a camera host: ISI port.
   a i2c camera sensor: ov2640 port.
 to sama5d3xmb.dtsi.
 
 In the ov2640 node, it defines the pinctrls, clocks and isi port.
 In the ISI node, it also reference to a ov2640 port.
 
 Signed-off-by: Josh Wu josh...@atmel.com
 ---
  arch/arm/boot/dts/sama5d3xmb.dtsi | 32 
  1 file changed, 32 insertions(+)
 
 diff --git a/arch/arm/boot/dts/sama5d3xmb.dtsi 
 b/arch/arm/boot/dts/sama5d3xmb.dtsi
 index 0aaebc6..958a528 100644
 --- a/arch/arm/boot/dts/sama5d3xmb.dtsi
 +++ b/arch/arm/boot/dts/sama5d3xmb.dtsi
 @@ -52,6 +52,29 @@
   };
   };
  
 + i2c1: i2c@f0018000 {
 + ov2640: camera@0x30 {
 + compatible = ovti,ov2640;
 + reg = 0x30;
 + pinctrl-names = default;
 + pinctrl-0 = pinctrl_isi_pck_as_mck 
 pinctrl_sensor_power pinctrl_sensor_reset;

I've acked your previous patch but maybe it should be named
pinctrl_isi_pck1_as_mck to be clearer (you used the handle to pck1
below).

 + resetb-gpios = pioE 24 
 GPIO_ACTIVE_LOW;
 + pwdn-gpios = pioE 29 
 GPIO_ACTIVE_HIGH;
 + /* use pck1 for the master clock of 
 ov2640 */
 + clocks = pck1;
 + clock-names = xvclk;
 + assigned-clocks = pck1;
 + assigned-clock-rates = 2500;
 +
 + port {
 + ov2640_0: endpoint {
 + remote-endpoint = 
 isi_0;
 + bus-width = 8;
 + };
 + };
 + };
 + };
 +
   usart1: serial@f002 {
   dmas = 0, 0;/*  Do not use DMA for 
 usart1 */
   pinctrl-names = default;
 @@ -60,6 +83,15 @@
   };
  
   isi: isi@f0034000 {
 + port {
 + #address-cells = 1;
 + #size-cells = 0;
 +
 + isi_0: endpoint {
 + remote-endpoint = ov2640_0;
 + bus-width = 8;
 + };
 + };
   };
  
   mmc1: mmc@f800 {
 -- 
 1.9.1
 

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 6/7] ARM: at91: dts: sama5d3: add ov2640 camera sensor support

2014-12-18 Thread Josh Wu
According to v4l2 dt document, we add:
  a camera host: ISI port.
  a i2c camera sensor: ov2640 port.
to sama5d3xmb.dtsi.

In the ov2640 node, it defines the pinctrls, clocks and isi port.
In the ISI node, it also reference to a ov2640 port.

Signed-off-by: Josh Wu josh...@atmel.com
---
 arch/arm/boot/dts/sama5d3xmb.dtsi | 32 
 1 file changed, 32 insertions(+)

diff --git a/arch/arm/boot/dts/sama5d3xmb.dtsi 
b/arch/arm/boot/dts/sama5d3xmb.dtsi
index 0aaebc6..958a528 100644
--- a/arch/arm/boot/dts/sama5d3xmb.dtsi
+++ b/arch/arm/boot/dts/sama5d3xmb.dtsi
@@ -52,6 +52,29 @@
};
};
 
+   i2c1: i2c@f0018000 {
+   ov2640: camera@0x30 {
+   compatible = ovti,ov2640;
+   reg = 0x30;
+   pinctrl-names = default;
+   pinctrl-0 = pinctrl_isi_pck_as_mck 
pinctrl_sensor_power pinctrl_sensor_reset;
+   resetb-gpios = pioE 24 
GPIO_ACTIVE_LOW;
+   pwdn-gpios = pioE 29 
GPIO_ACTIVE_HIGH;
+   /* use pck1 for the master clock of 
ov2640 */
+   clocks = pck1;
+   clock-names = xvclk;
+   assigned-clocks = pck1;
+   assigned-clock-rates = 2500;
+
+   port {
+   ov2640_0: endpoint {
+   remote-endpoint = 
isi_0;
+   bus-width = 8;
+   };
+   };
+   };
+   };
+
usart1: serial@f002 {
dmas = 0, 0;/*  Do not use DMA for 
usart1 */
pinctrl-names = default;
@@ -60,6 +83,15 @@
};
 
isi: isi@f0034000 {
+   port {
+   #address-cells = 1;
+   #size-cells = 0;
+
+   isi_0: endpoint {
+   remote-endpoint = ov2640_0;
+   bus-width = 8;
+   };
+   };
};
 
mmc1: mmc@f800 {
-- 
1.9.1

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 6/7] ARM: at91: dts: sama5d3: add ov2640 camera sensor support

2014-12-18 Thread Laurent Pinchart
Hi Josh,

Thank you for the patch.

On Thursday 18 December 2014 16:51:06 Josh Wu wrote:
 According to v4l2 dt document, we add:
   a camera host: ISI port.
   a i2c camera sensor: ov2640 port.
 to sama5d3xmb.dtsi.
 
 In the ov2640 node, it defines the pinctrls, clocks and isi port.
 In the ISI node, it also reference to a ov2640 port.
 
 Signed-off-by: Josh Wu josh...@atmel.com
 ---
  arch/arm/boot/dts/sama5d3xmb.dtsi | 32 
  1 file changed, 32 insertions(+)
 
 diff --git a/arch/arm/boot/dts/sama5d3xmb.dtsi
 b/arch/arm/boot/dts/sama5d3xmb.dtsi index 0aaebc6..958a528 100644
 --- a/arch/arm/boot/dts/sama5d3xmb.dtsi
 +++ b/arch/arm/boot/dts/sama5d3xmb.dtsi
 @@ -52,6 +52,29 @@
   };
   };
 
 + i2c1: i2c@f0018000 {
 + ov2640: camera@0x30 {
 + compatible = ovti,ov2640;
 + reg = 0x30;
 + pinctrl-names = default;
 + pinctrl-0 = pinctrl_isi_pck_as_mck
 pinctrl_sensor_power pinctrl_sensor_reset;
 + resetb-gpios = pioE 24 
 GPIO_ACTIVE_LOW;
 + pwdn-gpios = pioE 29 
 GPIO_ACTIVE_HIGH;
 + /* use pck1 for the master clock of 
 ov2640 */
 + clocks = pck1;
 + clock-names = xvclk;
 + assigned-clocks = pck1;
 + assigned-clock-rates = 2500;
 +
 + port {
 + ov2640_0: endpoint {
 + remote-endpoint = 
 isi_0;
 + bus-width = 8;
 + };
 + };
 + };
 + };
 +
   usart1: serial@f002 {
   dmas = 0, 0;/*  Do not use DMA for 
 usart1 */
   pinctrl-names = default;
 @@ -60,6 +83,15 @@
   };
 
   isi: isi@f0034000 {
 + port {
 + #address-cells = 1;
 + #size-cells = 0;
 +

I would add the port node and those two properties to 
arch/arm/boot/dts/sama5d3.dtsi, as the isi has a single port. The endpoint, of 
course, should stay in this file.

 + isi_0: endpoint {
 + remote-endpoint = ov2640_0;
 + bus-width = 8;
 + };
 + };
   };
 
   mmc1: mmc@f800 {

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 6/7] ARM: at91: dts: sama5d3: add ov2640 camera sensor support

2014-12-18 Thread Josh Wu


Hi, Laurent

On 12/18/2014 8:32 PM, Laurent Pinchart wrote:

Hi Josh,

Thank you for the patch.

On Thursday 18 December 2014 16:51:06 Josh Wu wrote:

According to v4l2 dt document, we add:
   a camera host: ISI port.
   a i2c camera sensor: ov2640 port.
to sama5d3xmb.dtsi.

In the ov2640 node, it defines the pinctrls, clocks and isi port.
In the ISI node, it also reference to a ov2640 port.

Signed-off-by: Josh Wu josh...@atmel.com
---
  arch/arm/boot/dts/sama5d3xmb.dtsi | 32 
  1 file changed, 32 insertions(+)

diff --git a/arch/arm/boot/dts/sama5d3xmb.dtsi
b/arch/arm/boot/dts/sama5d3xmb.dtsi index 0aaebc6..958a528 100644
--- a/arch/arm/boot/dts/sama5d3xmb.dtsi
+++ b/arch/arm/boot/dts/sama5d3xmb.dtsi
@@ -52,6 +52,29 @@
};
};

+   i2c1: i2c@f0018000 {
+   ov2640: camera@0x30 {
+   compatible = ovti,ov2640;
+   reg = 0x30;
+   pinctrl-names = default;
+   pinctrl-0 = pinctrl_isi_pck_as_mck
pinctrl_sensor_power pinctrl_sensor_reset;
+   resetb-gpios = pioE 24 
GPIO_ACTIVE_LOW;
+   pwdn-gpios = pioE 29 
GPIO_ACTIVE_HIGH;
+   /* use pck1 for the master clock of 
ov2640 */
+   clocks = pck1;
+   clock-names = xvclk;
+   assigned-clocks = pck1;
+   assigned-clock-rates = 2500;
+
+   port {
+   ov2640_0: endpoint {
+   remote-endpoint = 
isi_0;
+   bus-width = 8;
+   };
+   };
+   };
+   };
+
usart1: serial@f002 {
dmas = 0, 0;/*  Do not use DMA for 
usart1 */
pinctrl-names = default;
@@ -60,6 +83,15 @@
};

isi: isi@f0034000 {
+   port {
+   #address-cells = 1;
+   #size-cells = 0;
+

I would add the port node and those two properties to
arch/arm/boot/dts/sama5d3.dtsi, as the isi has a single port. The endpoint, of
course, should stay in this file.


That makes sense. I'll fix that. Thanks for the review.

Best Regards,
Josh Wu



+   isi_0: endpoint {
+   remote-endpoint = ov2640_0;
+   bus-width = 8;
+   };
+   };
};

mmc1: mmc@f800 {


--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html