All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] ARM: dts: dra76-evm: remove ov5640
@ 2021-04-12  7:53 ` Tomi Valkeinen
  0 siblings, 0 replies; 18+ messages in thread
From: Tomi Valkeinen @ 2021-04-12  7:53 UTC (permalink / raw)
  To: Nishanth Menon, Benoit Parrot, Laurent Pinchart, devicetree,
	Rob Herring, linux-arm-kernel, Tony Lindgren, Pratyush Yadav
  Cc: Tomi Valkeinen

DRA76 EVM boards are not shipped with OV5640 sensor module, it is a
separate purchase. OV5640 module is also just one of the possible
sensors or capture boards you can connect.

However, for some reason, OV5640 has been added to the board dts file,
making it cumbersome to use other sensors.

Remove the OV5640 from the dts file so that it is easy to use other
sensors via DT overlays.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 arch/arm/boot/dts/dra76-evm.dts | 27 ---------------------------
 1 file changed, 27 deletions(-)

diff --git a/arch/arm/boot/dts/dra76-evm.dts b/arch/arm/boot/dts/dra76-evm.dts
index 9bd01ae40b1d..f3c900e1b953 100644
--- a/arch/arm/boot/dts/dra76-evm.dts
+++ b/arch/arm/boot/dts/dra76-evm.dts
@@ -158,12 +158,6 @@ aic_dvdd: fixedregulator-aic_dvdd {
 		regulator-max-microvolt = <1800000>;
 	};
 
-	clk_ov5640_fixed: clock {
-		compatible = "fixed-clock";
-		#clock-cells = <0>;
-		clock-frequency = <24000000>;
-	};
-
 	hdmi0: connector {
 		compatible = "hdmi-connector";
 		label = "hdmi";
@@ -409,22 +403,6 @@ tlv320aic3106: tlv320aic3106@19 {
 &i2c5 {
 	status = "okay";
 	clock-frequency = <400000>;
-
-	ov5640@3c {
-		compatible = "ovti,ov5640";
-		reg = <0x3c>;
-
-		clocks = <&clk_ov5640_fixed>;
-		clock-names = "xclk";
-
-		port {
-			csi2_cam0: endpoint {
-				remote-endpoint = <&csi2_phy0>;
-				clock-lanes = <0>;
-				data-lanes = <1 2>;
-			};
-		};
-	};
 };
 
 &cpu0 {
@@ -574,11 +552,6 @@ can-transceiver {
 };
 
 &csi2_0 {
-	csi2_phy0: endpoint {
-		remote-endpoint = <&csi2_cam0>;
-		clock-lanes = <0>;
-		data-lanes = <1 2>;
-	};
 };
 
 &ipu2 {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 1/2] ARM: dts: dra76-evm: remove ov5640
@ 2021-04-12  7:53 ` Tomi Valkeinen
  0 siblings, 0 replies; 18+ messages in thread
From: Tomi Valkeinen @ 2021-04-12  7:53 UTC (permalink / raw)
  To: Nishanth Menon, Benoit Parrot, Laurent Pinchart, devicetree,
	Rob Herring, linux-arm-kernel, Tony Lindgren, Pratyush Yadav
  Cc: Tomi Valkeinen

DRA76 EVM boards are not shipped with OV5640 sensor module, it is a
separate purchase. OV5640 module is also just one of the possible
sensors or capture boards you can connect.

However, for some reason, OV5640 has been added to the board dts file,
making it cumbersome to use other sensors.

Remove the OV5640 from the dts file so that it is easy to use other
sensors via DT overlays.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 arch/arm/boot/dts/dra76-evm.dts | 27 ---------------------------
 1 file changed, 27 deletions(-)

diff --git a/arch/arm/boot/dts/dra76-evm.dts b/arch/arm/boot/dts/dra76-evm.dts
index 9bd01ae40b1d..f3c900e1b953 100644
--- a/arch/arm/boot/dts/dra76-evm.dts
+++ b/arch/arm/boot/dts/dra76-evm.dts
@@ -158,12 +158,6 @@ aic_dvdd: fixedregulator-aic_dvdd {
 		regulator-max-microvolt = <1800000>;
 	};
 
-	clk_ov5640_fixed: clock {
-		compatible = "fixed-clock";
-		#clock-cells = <0>;
-		clock-frequency = <24000000>;
-	};
-
 	hdmi0: connector {
 		compatible = "hdmi-connector";
 		label = "hdmi";
@@ -409,22 +403,6 @@ tlv320aic3106: tlv320aic3106@19 {
 &i2c5 {
 	status = "okay";
 	clock-frequency = <400000>;
-
-	ov5640@3c {
-		compatible = "ovti,ov5640";
-		reg = <0x3c>;
-
-		clocks = <&clk_ov5640_fixed>;
-		clock-names = "xclk";
-
-		port {
-			csi2_cam0: endpoint {
-				remote-endpoint = <&csi2_phy0>;
-				clock-lanes = <0>;
-				data-lanes = <1 2>;
-			};
-		};
-	};
 };
 
 &cpu0 {
@@ -574,11 +552,6 @@ can-transceiver {
 };
 
 &csi2_0 {
-	csi2_phy0: endpoint {
-		remote-endpoint = <&csi2_cam0>;
-		clock-lanes = <0>;
-		data-lanes = <1 2>;
-	};
 };
 
 &ipu2 {
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/2] arm64: dts: ti: k3-am654-base-board: remove ov5640
  2021-04-12  7:53 ` Tomi Valkeinen
@ 2021-04-12  7:53   ` Tomi Valkeinen
  -1 siblings, 0 replies; 18+ messages in thread
From: Tomi Valkeinen @ 2021-04-12  7:53 UTC (permalink / raw)
  To: Nishanth Menon, Benoit Parrot, Laurent Pinchart, devicetree,
	Rob Herring, linux-arm-kernel, Tony Lindgren, Pratyush Yadav
  Cc: Tomi Valkeinen

AM654 EVM boards are not shipped with OV5640 sensor module, it is a
separate purchase. OV5640 module is also just one of the possible
sensors or capture boards you can connect.

However, for some reason, OV5640 has been added to the board dts file,
making it cumbersome to use other sensors.

Remove the OV5640 from the dts file so that it is easy to use other
sensors via DT overlays.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 .../arm64/boot/dts/ti/k3-am654-base-board.dts | 27 -------------------
 1 file changed, 27 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
index fe3043943906..76358b4944e1 100644
--- a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
+++ b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
@@ -85,12 +85,6 @@ sw6 {
 			gpios = <&wkup_gpio0 27 GPIO_ACTIVE_LOW>;
 		};
 	};
-
-	clk_ov5640_fixed: clock {
-		compatible = "fixed-clock";
-		#clock-cells = <0>;
-		clock-frequency = <24000000>;
-	};
 };
 
 &wkup_pmx0 {
@@ -288,22 +282,6 @@ &main_i2c1 {
 	pinctrl-0 = <&main_i2c1_pins_default>;
 	clock-frequency = <400000>;
 
-	ov5640: camera@3c {
-		compatible = "ovti,ov5640";
-		reg = <0x3c>;
-
-		clocks = <&clk_ov5640_fixed>;
-		clock-names = "xclk";
-
-		port {
-			csi2_cam0: endpoint {
-				remote-endpoint = <&csi2_phy0>;
-				clock-lanes = <0>;
-				data-lanes = <1 2>;
-			};
-		};
-	};
-
 };
 
 &main_i2c2 {
@@ -497,11 +475,6 @@ flash@0{
 };
 
 &csi2_0 {
-	csi2_phy0: endpoint {
-		remote-endpoint = <&csi2_cam0>;
-		clock-lanes = <0>;
-		data-lanes = <1 2>;
-	};
 };
 
 &mcu_cpsw {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/2] arm64: dts: ti: k3-am654-base-board: remove ov5640
@ 2021-04-12  7:53   ` Tomi Valkeinen
  0 siblings, 0 replies; 18+ messages in thread
From: Tomi Valkeinen @ 2021-04-12  7:53 UTC (permalink / raw)
  To: Nishanth Menon, Benoit Parrot, Laurent Pinchart, devicetree,
	Rob Herring, linux-arm-kernel, Tony Lindgren, Pratyush Yadav
  Cc: Tomi Valkeinen

AM654 EVM boards are not shipped with OV5640 sensor module, it is a
separate purchase. OV5640 module is also just one of the possible
sensors or capture boards you can connect.

However, for some reason, OV5640 has been added to the board dts file,
making it cumbersome to use other sensors.

Remove the OV5640 from the dts file so that it is easy to use other
sensors via DT overlays.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 .../arm64/boot/dts/ti/k3-am654-base-board.dts | 27 -------------------
 1 file changed, 27 deletions(-)

diff --git a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
index fe3043943906..76358b4944e1 100644
--- a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
+++ b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
@@ -85,12 +85,6 @@ sw6 {
 			gpios = <&wkup_gpio0 27 GPIO_ACTIVE_LOW>;
 		};
 	};
-
-	clk_ov5640_fixed: clock {
-		compatible = "fixed-clock";
-		#clock-cells = <0>;
-		clock-frequency = <24000000>;
-	};
 };
 
 &wkup_pmx0 {
@@ -288,22 +282,6 @@ &main_i2c1 {
 	pinctrl-0 = <&main_i2c1_pins_default>;
 	clock-frequency = <400000>;
 
-	ov5640: camera@3c {
-		compatible = "ovti,ov5640";
-		reg = <0x3c>;
-
-		clocks = <&clk_ov5640_fixed>;
-		clock-names = "xclk";
-
-		port {
-			csi2_cam0: endpoint {
-				remote-endpoint = <&csi2_phy0>;
-				clock-lanes = <0>;
-				data-lanes = <1 2>;
-			};
-		};
-	};
-
 };
 
 &main_i2c2 {
@@ -497,11 +475,6 @@ flash@0{
 };
 
 &csi2_0 {
-	csi2_phy0: endpoint {
-		remote-endpoint = <&csi2_cam0>;
-		clock-lanes = <0>;
-		data-lanes = <1 2>;
-	};
 };
 
 &mcu_cpsw {
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] ARM: dts: dra76-evm: remove ov5640
  2021-04-12  7:53 ` Tomi Valkeinen
@ 2021-04-12  8:00   ` Laurent Pinchart
  -1 siblings, 0 replies; 18+ messages in thread
From: Laurent Pinchart @ 2021-04-12  8:00 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Nishanth Menon, Benoit Parrot, devicetree, Rob Herring,
	linux-arm-kernel, Tony Lindgren, Pratyush Yadav

Hi Tomi,

Thank you for the patch.

On Mon, Apr 12, 2021 at 10:53:05AM +0300, Tomi Valkeinen wrote:
> DRA76 EVM boards are not shipped with OV5640 sensor module, it is a
> separate purchase. OV5640 module is also just one of the possible
> sensors or capture boards you can connect.
> 
> However, for some reason, OV5640 has been added to the board dts file,
> making it cumbersome to use other sensors.
> 
> Remove the OV5640 from the dts file so that it is easy to use other
> sensors via DT overlays.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  arch/arm/boot/dts/dra76-evm.dts | 27 ---------------------------
>  1 file changed, 27 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/dra76-evm.dts b/arch/arm/boot/dts/dra76-evm.dts
> index 9bd01ae40b1d..f3c900e1b953 100644
> --- a/arch/arm/boot/dts/dra76-evm.dts
> +++ b/arch/arm/boot/dts/dra76-evm.dts
> @@ -158,12 +158,6 @@ aic_dvdd: fixedregulator-aic_dvdd {
>  		regulator-max-microvolt = <1800000>;
>  	};
>  
> -	clk_ov5640_fixed: clock {
> -		compatible = "fixed-clock";
> -		#clock-cells = <0>;
> -		clock-frequency = <24000000>;
> -	};
> -
>  	hdmi0: connector {
>  		compatible = "hdmi-connector";
>  		label = "hdmi";
> @@ -409,22 +403,6 @@ tlv320aic3106: tlv320aic3106@19 {
>  &i2c5 {
>  	status = "okay";
>  	clock-frequency = <400000>;
> -
> -	ov5640@3c {
> -		compatible = "ovti,ov5640";
> -		reg = <0x3c>;
> -
> -		clocks = <&clk_ov5640_fixed>;
> -		clock-names = "xclk";
> -
> -		port {
> -			csi2_cam0: endpoint {
> -				remote-endpoint = <&csi2_phy0>;
> -				clock-lanes = <0>;
> -				data-lanes = <1 2>;
> -			};
> -		};
> -	};
>  };

You could drop the node completely.

>  
>  &cpu0 {
> @@ -574,11 +552,6 @@ can-transceiver {
>  };
>  
>  &csi2_0 {
> -	csi2_phy0: endpoint {
> -		remote-endpoint = <&csi2_cam0>;
> -		clock-lanes = <0>;
> -		data-lanes = <1 2>;
> -	};
>  };

And here too.

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

Is there an overlay available somewhere for this ?

>  
>  &ipu2 {

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] ARM: dts: dra76-evm: remove ov5640
@ 2021-04-12  8:00   ` Laurent Pinchart
  0 siblings, 0 replies; 18+ messages in thread
From: Laurent Pinchart @ 2021-04-12  8:00 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Nishanth Menon, Benoit Parrot, devicetree, Rob Herring,
	linux-arm-kernel, Tony Lindgren, Pratyush Yadav

Hi Tomi,

Thank you for the patch.

On Mon, Apr 12, 2021 at 10:53:05AM +0300, Tomi Valkeinen wrote:
> DRA76 EVM boards are not shipped with OV5640 sensor module, it is a
> separate purchase. OV5640 module is also just one of the possible
> sensors or capture boards you can connect.
> 
> However, for some reason, OV5640 has been added to the board dts file,
> making it cumbersome to use other sensors.
> 
> Remove the OV5640 from the dts file so that it is easy to use other
> sensors via DT overlays.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  arch/arm/boot/dts/dra76-evm.dts | 27 ---------------------------
>  1 file changed, 27 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/dra76-evm.dts b/arch/arm/boot/dts/dra76-evm.dts
> index 9bd01ae40b1d..f3c900e1b953 100644
> --- a/arch/arm/boot/dts/dra76-evm.dts
> +++ b/arch/arm/boot/dts/dra76-evm.dts
> @@ -158,12 +158,6 @@ aic_dvdd: fixedregulator-aic_dvdd {
>  		regulator-max-microvolt = <1800000>;
>  	};
>  
> -	clk_ov5640_fixed: clock {
> -		compatible = "fixed-clock";
> -		#clock-cells = <0>;
> -		clock-frequency = <24000000>;
> -	};
> -
>  	hdmi0: connector {
>  		compatible = "hdmi-connector";
>  		label = "hdmi";
> @@ -409,22 +403,6 @@ tlv320aic3106: tlv320aic3106@19 {
>  &i2c5 {
>  	status = "okay";
>  	clock-frequency = <400000>;
> -
> -	ov5640@3c {
> -		compatible = "ovti,ov5640";
> -		reg = <0x3c>;
> -
> -		clocks = <&clk_ov5640_fixed>;
> -		clock-names = "xclk";
> -
> -		port {
> -			csi2_cam0: endpoint {
> -				remote-endpoint = <&csi2_phy0>;
> -				clock-lanes = <0>;
> -				data-lanes = <1 2>;
> -			};
> -		};
> -	};
>  };

You could drop the node completely.

>  
>  &cpu0 {
> @@ -574,11 +552,6 @@ can-transceiver {
>  };
>  
>  &csi2_0 {
> -	csi2_phy0: endpoint {
> -		remote-endpoint = <&csi2_cam0>;
> -		clock-lanes = <0>;
> -		data-lanes = <1 2>;
> -	};
>  };

And here too.

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

Is there an overlay available somewhere for this ?

>  
>  &ipu2 {

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] arm64: dts: ti: k3-am654-base-board: remove ov5640
  2021-04-12  7:53   ` Tomi Valkeinen
@ 2021-04-12  8:00     ` Laurent Pinchart
  -1 siblings, 0 replies; 18+ messages in thread
From: Laurent Pinchart @ 2021-04-12  8:00 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Nishanth Menon, Benoit Parrot, devicetree, Rob Herring,
	linux-arm-kernel, Tony Lindgren, Pratyush Yadav

Hi Tomi,

Thank you for the patch.

On Mon, Apr 12, 2021 at 10:53:06AM +0300, Tomi Valkeinen wrote:
> AM654 EVM boards are not shipped with OV5640 sensor module, it is a
> separate purchase. OV5640 module is also just one of the possible
> sensors or capture boards you can connect.
> 
> However, for some reason, OV5640 has been added to the board dts file,
> making it cumbersome to use other sensors.
> 
> Remove the OV5640 from the dts file so that it is easy to use other
> sensors via DT overlays.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  .../arm64/boot/dts/ti/k3-am654-base-board.dts | 27 -------------------
>  1 file changed, 27 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
> index fe3043943906..76358b4944e1 100644
> --- a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
> +++ b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
> @@ -85,12 +85,6 @@ sw6 {
>  			gpios = <&wkup_gpio0 27 GPIO_ACTIVE_LOW>;
>  		};
>  	};
> -
> -	clk_ov5640_fixed: clock {
> -		compatible = "fixed-clock";
> -		#clock-cells = <0>;
> -		clock-frequency = <24000000>;
> -	};
>  };
>  
>  &wkup_pmx0 {
> @@ -288,22 +282,6 @@ &main_i2c1 {
>  	pinctrl-0 = <&main_i2c1_pins_default>;
>  	clock-frequency = <400000>;
>  
> -	ov5640: camera@3c {
> -		compatible = "ovti,ov5640";
> -		reg = <0x3c>;
> -
> -		clocks = <&clk_ov5640_fixed>;
> -		clock-names = "xclk";
> -
> -		port {
> -			csi2_cam0: endpoint {
> -				remote-endpoint = <&csi2_phy0>;
> -				clock-lanes = <0>;
> -				data-lanes = <1 2>;
> -			};
> -		};
> -	};
> -
>  };

As for patch 1/2, you could drop the two nodes completely. Same question
about overlay availability.

>  
>  &main_i2c2 {
> @@ -497,11 +475,6 @@ flash@0{
>  };
>  
>  &csi2_0 {
> -	csi2_phy0: endpoint {
> -		remote-endpoint = <&csi2_cam0>;
> -		clock-lanes = <0>;
> -		data-lanes = <1 2>;
> -	};
>  };
>  
>  &mcu_cpsw {

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] arm64: dts: ti: k3-am654-base-board: remove ov5640
@ 2021-04-12  8:00     ` Laurent Pinchart
  0 siblings, 0 replies; 18+ messages in thread
From: Laurent Pinchart @ 2021-04-12  8:00 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Nishanth Menon, Benoit Parrot, devicetree, Rob Herring,
	linux-arm-kernel, Tony Lindgren, Pratyush Yadav

Hi Tomi,

Thank you for the patch.

On Mon, Apr 12, 2021 at 10:53:06AM +0300, Tomi Valkeinen wrote:
> AM654 EVM boards are not shipped with OV5640 sensor module, it is a
> separate purchase. OV5640 module is also just one of the possible
> sensors or capture boards you can connect.
> 
> However, for some reason, OV5640 has been added to the board dts file,
> making it cumbersome to use other sensors.
> 
> Remove the OV5640 from the dts file so that it is easy to use other
> sensors via DT overlays.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  .../arm64/boot/dts/ti/k3-am654-base-board.dts | 27 -------------------
>  1 file changed, 27 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
> index fe3043943906..76358b4944e1 100644
> --- a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
> +++ b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
> @@ -85,12 +85,6 @@ sw6 {
>  			gpios = <&wkup_gpio0 27 GPIO_ACTIVE_LOW>;
>  		};
>  	};
> -
> -	clk_ov5640_fixed: clock {
> -		compatible = "fixed-clock";
> -		#clock-cells = <0>;
> -		clock-frequency = <24000000>;
> -	};
>  };
>  
>  &wkup_pmx0 {
> @@ -288,22 +282,6 @@ &main_i2c1 {
>  	pinctrl-0 = <&main_i2c1_pins_default>;
>  	clock-frequency = <400000>;
>  
> -	ov5640: camera@3c {
> -		compatible = "ovti,ov5640";
> -		reg = <0x3c>;
> -
> -		clocks = <&clk_ov5640_fixed>;
> -		clock-names = "xclk";
> -
> -		port {
> -			csi2_cam0: endpoint {
> -				remote-endpoint = <&csi2_phy0>;
> -				clock-lanes = <0>;
> -				data-lanes = <1 2>;
> -			};
> -		};
> -	};
> -
>  };

As for patch 1/2, you could drop the two nodes completely. Same question
about overlay availability.

>  
>  &main_i2c2 {
> @@ -497,11 +475,6 @@ flash@0{
>  };
>  
>  &csi2_0 {
> -	csi2_phy0: endpoint {
> -		remote-endpoint = <&csi2_cam0>;
> -		clock-lanes = <0>;
> -		data-lanes = <1 2>;
> -	};
>  };
>  
>  &mcu_cpsw {

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] ARM: dts: dra76-evm: remove ov5640
  2021-04-12  8:00   ` Laurent Pinchart
@ 2021-04-12  8:26     ` Tomi Valkeinen
  -1 siblings, 0 replies; 18+ messages in thread
From: Tomi Valkeinen @ 2021-04-12  8:26 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Nishanth Menon, Benoit Parrot, devicetree, Rob Herring,
	linux-arm-kernel, Tony Lindgren, Pratyush Yadav

On 12/04/2021 11:00, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Mon, Apr 12, 2021 at 10:53:05AM +0300, Tomi Valkeinen wrote:
>> DRA76 EVM boards are not shipped with OV5640 sensor module, it is a
>> separate purchase. OV5640 module is also just one of the possible
>> sensors or capture boards you can connect.
>>
>> However, for some reason, OV5640 has been added to the board dts file,
>> making it cumbersome to use other sensors.
>>
>> Remove the OV5640 from the dts file so that it is easy to use other
>> sensors via DT overlays.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>> ---
>>   arch/arm/boot/dts/dra76-evm.dts | 27 ---------------------------
>>   1 file changed, 27 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/dra76-evm.dts b/arch/arm/boot/dts/dra76-evm.dts
>> index 9bd01ae40b1d..f3c900e1b953 100644
>> --- a/arch/arm/boot/dts/dra76-evm.dts
>> +++ b/arch/arm/boot/dts/dra76-evm.dts
>> @@ -158,12 +158,6 @@ aic_dvdd: fixedregulator-aic_dvdd {
>>   		regulator-max-microvolt = <1800000>;
>>   	};
>>   
>> -	clk_ov5640_fixed: clock {
>> -		compatible = "fixed-clock";
>> -		#clock-cells = <0>;
>> -		clock-frequency = <24000000>;
>> -	};
>> -
>>   	hdmi0: connector {
>>   		compatible = "hdmi-connector";
>>   		label = "hdmi";
>> @@ -409,22 +403,6 @@ tlv320aic3106: tlv320aic3106@19 {
>>   &i2c5 {
>>   	status = "okay";
>>   	clock-frequency = <400000>;
>> -
>> -	ov5640@3c {
>> -		compatible = "ovti,ov5640";
>> -		reg = <0x3c>;
>> -
>> -		clocks = <&clk_ov5640_fixed>;
>> -		clock-names = "xclk";
>> -
>> -		port {
>> -			csi2_cam0: endpoint {
>> -				remote-endpoint = <&csi2_phy0>;
>> -				clock-lanes = <0>;
>> -				data-lanes = <1 2>;
>> -			};
>> -		};
>> -	};
>>   };
> 
> You could drop the node completely.
> 
>>   
>>   &cpu0 {
>> @@ -574,11 +552,6 @@ can-transceiver {
>>   };
>>   
>>   &csi2_0 {
>> -	csi2_phy0: endpoint {
>> -		remote-endpoint = <&csi2_cam0>;
>> -		clock-lanes = <0>;
>> -		data-lanes = <1 2>;
>> -	};
>>   };
> 
> And here too.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Is there an overlay available somewhere for this ?

I have a work branch for dra76 and am654 overlays for TI's DT overlay tree:

https://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux-dt.git/commit/?h=multistream/work&id=c997e4854164cbfcf11174e9c7d012967b6f399d
https://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux-dt.git/commit/?h=multistream/work&id=311bdaaa28ba16b0142f5879fc7c79aa3aa5906d

I have not sent these to TI yet.

  Tomi

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] ARM: dts: dra76-evm: remove ov5640
@ 2021-04-12  8:26     ` Tomi Valkeinen
  0 siblings, 0 replies; 18+ messages in thread
From: Tomi Valkeinen @ 2021-04-12  8:26 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Nishanth Menon, Benoit Parrot, devicetree, Rob Herring,
	linux-arm-kernel, Tony Lindgren, Pratyush Yadav

On 12/04/2021 11:00, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Mon, Apr 12, 2021 at 10:53:05AM +0300, Tomi Valkeinen wrote:
>> DRA76 EVM boards are not shipped with OV5640 sensor module, it is a
>> separate purchase. OV5640 module is also just one of the possible
>> sensors or capture boards you can connect.
>>
>> However, for some reason, OV5640 has been added to the board dts file,
>> making it cumbersome to use other sensors.
>>
>> Remove the OV5640 from the dts file so that it is easy to use other
>> sensors via DT overlays.
>>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>> ---
>>   arch/arm/boot/dts/dra76-evm.dts | 27 ---------------------------
>>   1 file changed, 27 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/dra76-evm.dts b/arch/arm/boot/dts/dra76-evm.dts
>> index 9bd01ae40b1d..f3c900e1b953 100644
>> --- a/arch/arm/boot/dts/dra76-evm.dts
>> +++ b/arch/arm/boot/dts/dra76-evm.dts
>> @@ -158,12 +158,6 @@ aic_dvdd: fixedregulator-aic_dvdd {
>>   		regulator-max-microvolt = <1800000>;
>>   	};
>>   
>> -	clk_ov5640_fixed: clock {
>> -		compatible = "fixed-clock";
>> -		#clock-cells = <0>;
>> -		clock-frequency = <24000000>;
>> -	};
>> -
>>   	hdmi0: connector {
>>   		compatible = "hdmi-connector";
>>   		label = "hdmi";
>> @@ -409,22 +403,6 @@ tlv320aic3106: tlv320aic3106@19 {
>>   &i2c5 {
>>   	status = "okay";
>>   	clock-frequency = <400000>;
>> -
>> -	ov5640@3c {
>> -		compatible = "ovti,ov5640";
>> -		reg = <0x3c>;
>> -
>> -		clocks = <&clk_ov5640_fixed>;
>> -		clock-names = "xclk";
>> -
>> -		port {
>> -			csi2_cam0: endpoint {
>> -				remote-endpoint = <&csi2_phy0>;
>> -				clock-lanes = <0>;
>> -				data-lanes = <1 2>;
>> -			};
>> -		};
>> -	};
>>   };
> 
> You could drop the node completely.
> 
>>   
>>   &cpu0 {
>> @@ -574,11 +552,6 @@ can-transceiver {
>>   };
>>   
>>   &csi2_0 {
>> -	csi2_phy0: endpoint {
>> -		remote-endpoint = <&csi2_cam0>;
>> -		clock-lanes = <0>;
>> -		data-lanes = <1 2>;
>> -	};
>>   };
> 
> And here too.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Is there an overlay available somewhere for this ?

I have a work branch for dra76 and am654 overlays for TI's DT overlay tree:

https://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux-dt.git/commit/?h=multistream/work&id=c997e4854164cbfcf11174e9c7d012967b6f399d
https://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux-dt.git/commit/?h=multistream/work&id=311bdaaa28ba16b0142f5879fc7c79aa3aa5906d

I have not sent these to TI yet.

  Tomi

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] arm64: dts: ti: k3-am654-base-board: remove ov5640
  2021-04-12  8:00     ` Laurent Pinchart
@ 2021-04-12  8:36       ` Pratyush Yadav
  -1 siblings, 0 replies; 18+ messages in thread
From: Pratyush Yadav @ 2021-04-12  8:36 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Tomi Valkeinen, Nishanth Menon, Benoit Parrot, devicetree,
	Rob Herring, linux-arm-kernel, Tony Lindgren,
	Vignesh Raghavendra

+ Vignesh

On 12/04/21 11:00AM, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Mon, Apr 12, 2021 at 10:53:06AM +0300, Tomi Valkeinen wrote:
> > AM654 EVM boards are not shipped with OV5640 sensor module, it is a
> > separate purchase. OV5640 module is also just one of the possible
> > sensors or capture boards you can connect.
> > 
> > However, for some reason, OV5640 has been added to the board dts file,
> > making it cumbersome to use other sensors.
> > 
> > Remove the OV5640 from the dts file so that it is easy to use other
> > sensors via DT overlays.
> > 
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> > ---
> >  .../arm64/boot/dts/ti/k3-am654-base-board.dts | 27 -------------------
> >  1 file changed, 27 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
> > index fe3043943906..76358b4944e1 100644
> > --- a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
> > +++ b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
> > @@ -85,12 +85,6 @@ sw6 {
> >  			gpios = <&wkup_gpio0 27 GPIO_ACTIVE_LOW>;
> >  		};
> >  	};
> > -
> > -	clk_ov5640_fixed: clock {
> > -		compatible = "fixed-clock";
> > -		#clock-cells = <0>;
> > -		clock-frequency = <24000000>;
> > -	};
> >  };
> >  
> >  &wkup_pmx0 {
> > @@ -288,22 +282,6 @@ &main_i2c1 {
> >  	pinctrl-0 = <&main_i2c1_pins_default>;
> >  	clock-frequency = <400000>;
> >  
> > -	ov5640: camera@3c {
> > -		compatible = "ovti,ov5640";
> > -		reg = <0x3c>;
> > -
> > -		clocks = <&clk_ov5640_fixed>;
> > -		clock-names = "xclk";
> > -
> > -		port {
> > -			csi2_cam0: endpoint {
> > -				remote-endpoint = <&csi2_phy0>;
> > -				clock-lanes = <0>;
> > -				data-lanes = <1 2>;
> > -			};
> > -		};
> > -	};
> > -
> >  };
> 
> As for patch 1/2, you could drop the two nodes completely. Same question
> about overlay availability.

The &main_i2c1 node was added much before the OV5640 node in 
19a1768fc34a (arm64: dts: ti: k3-am654-base-board: Add I2C nodes, 
2018-11-13). I wonder if there is any reason for having it present even 
if there are no subnodes. One reason that I can think of is that this 
node defines the pinmux configuration and clock frequency which makes 
more sense here than in an overlay.

> 
> >  
> >  &main_i2c2 {
> > @@ -497,11 +475,6 @@ flash@0{
> >  };
> >  
> >  &csi2_0 {
> > -	csi2_phy0: endpoint {
> > -		remote-endpoint = <&csi2_cam0>;
> > -		clock-lanes = <0>;
> > -		data-lanes = <1 2>;
> > -	};
> >  };

I agree with Laurent that the entire &csi2_0 node can be dropped.

Have you tested the CAL driver with this node removed and no overlay to 
add it back? Can it handle the error gracefully or does it crash and 
burn?

> >  
> >  &mcu_cpsw {
> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] arm64: dts: ti: k3-am654-base-board: remove ov5640
@ 2021-04-12  8:36       ` Pratyush Yadav
  0 siblings, 0 replies; 18+ messages in thread
From: Pratyush Yadav @ 2021-04-12  8:36 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Tomi Valkeinen, Nishanth Menon, Benoit Parrot, devicetree,
	Rob Herring, linux-arm-kernel, Tony Lindgren,
	Vignesh Raghavendra

+ Vignesh

On 12/04/21 11:00AM, Laurent Pinchart wrote:
> Hi Tomi,
> 
> Thank you for the patch.
> 
> On Mon, Apr 12, 2021 at 10:53:06AM +0300, Tomi Valkeinen wrote:
> > AM654 EVM boards are not shipped with OV5640 sensor module, it is a
> > separate purchase. OV5640 module is also just one of the possible
> > sensors or capture boards you can connect.
> > 
> > However, for some reason, OV5640 has been added to the board dts file,
> > making it cumbersome to use other sensors.
> > 
> > Remove the OV5640 from the dts file so that it is easy to use other
> > sensors via DT overlays.
> > 
> > Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> > ---
> >  .../arm64/boot/dts/ti/k3-am654-base-board.dts | 27 -------------------
> >  1 file changed, 27 deletions(-)
> > 
> > diff --git a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
> > index fe3043943906..76358b4944e1 100644
> > --- a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
> > +++ b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
> > @@ -85,12 +85,6 @@ sw6 {
> >  			gpios = <&wkup_gpio0 27 GPIO_ACTIVE_LOW>;
> >  		};
> >  	};
> > -
> > -	clk_ov5640_fixed: clock {
> > -		compatible = "fixed-clock";
> > -		#clock-cells = <0>;
> > -		clock-frequency = <24000000>;
> > -	};
> >  };
> >  
> >  &wkup_pmx0 {
> > @@ -288,22 +282,6 @@ &main_i2c1 {
> >  	pinctrl-0 = <&main_i2c1_pins_default>;
> >  	clock-frequency = <400000>;
> >  
> > -	ov5640: camera@3c {
> > -		compatible = "ovti,ov5640";
> > -		reg = <0x3c>;
> > -
> > -		clocks = <&clk_ov5640_fixed>;
> > -		clock-names = "xclk";
> > -
> > -		port {
> > -			csi2_cam0: endpoint {
> > -				remote-endpoint = <&csi2_phy0>;
> > -				clock-lanes = <0>;
> > -				data-lanes = <1 2>;
> > -			};
> > -		};
> > -	};
> > -
> >  };
> 
> As for patch 1/2, you could drop the two nodes completely. Same question
> about overlay availability.

The &main_i2c1 node was added much before the OV5640 node in 
19a1768fc34a (arm64: dts: ti: k3-am654-base-board: Add I2C nodes, 
2018-11-13). I wonder if there is any reason for having it present even 
if there are no subnodes. One reason that I can think of is that this 
node defines the pinmux configuration and clock frequency which makes 
more sense here than in an overlay.

> 
> >  
> >  &main_i2c2 {
> > @@ -497,11 +475,6 @@ flash@0{
> >  };
> >  
> >  &csi2_0 {
> > -	csi2_phy0: endpoint {
> > -		remote-endpoint = <&csi2_cam0>;
> > -		clock-lanes = <0>;
> > -		data-lanes = <1 2>;
> > -	};
> >  };

I agree with Laurent that the entire &csi2_0 node can be dropped.

Have you tested the CAL driver with this node removed and no overlay to 
add it back? Can it handle the error gracefully or does it crash and 
burn?

> >  
> >  &mcu_cpsw {
> 
> -- 
> Regards,
> 
> Laurent Pinchart

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] arm64: dts: ti: k3-am654-base-board: remove ov5640
  2021-04-12  8:36       ` Pratyush Yadav
@ 2021-04-12  9:15         ` Tomi Valkeinen
  -1 siblings, 0 replies; 18+ messages in thread
From: Tomi Valkeinen @ 2021-04-12  9:15 UTC (permalink / raw)
  To: Pratyush Yadav, Laurent Pinchart
  Cc: Nishanth Menon, Benoit Parrot, devicetree, Rob Herring,
	linux-arm-kernel, Tony Lindgren, Vignesh Raghavendra

On 12/04/2021 11:36, Pratyush Yadav wrote:
> + Vignesh
> 
> On 12/04/21 11:00AM, Laurent Pinchart wrote:
>> Hi Tomi,
>>
>> Thank you for the patch.
>>
>> On Mon, Apr 12, 2021 at 10:53:06AM +0300, Tomi Valkeinen wrote:
>>> AM654 EVM boards are not shipped with OV5640 sensor module, it is a
>>> separate purchase. OV5640 module is also just one of the possible
>>> sensors or capture boards you can connect.
>>>
>>> However, for some reason, OV5640 has been added to the board dts file,
>>> making it cumbersome to use other sensors.
>>>
>>> Remove the OV5640 from the dts file so that it is easy to use other
>>> sensors via DT overlays.
>>>
>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>>> ---
>>>   .../arm64/boot/dts/ti/k3-am654-base-board.dts | 27 -------------------
>>>   1 file changed, 27 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
>>> index fe3043943906..76358b4944e1 100644
>>> --- a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
>>> +++ b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
>>> @@ -85,12 +85,6 @@ sw6 {
>>>   			gpios = <&wkup_gpio0 27 GPIO_ACTIVE_LOW>;
>>>   		};
>>>   	};
>>> -
>>> -	clk_ov5640_fixed: clock {
>>> -		compatible = "fixed-clock";
>>> -		#clock-cells = <0>;
>>> -		clock-frequency = <24000000>;
>>> -	};
>>>   };
>>>   
>>>   &wkup_pmx0 {
>>> @@ -288,22 +282,6 @@ &main_i2c1 {
>>>   	pinctrl-0 = <&main_i2c1_pins_default>;
>>>   	clock-frequency = <400000>;
>>>   
>>> -	ov5640: camera@3c {
>>> -		compatible = "ovti,ov5640";
>>> -		reg = <0x3c>;
>>> -
>>> -		clocks = <&clk_ov5640_fixed>;
>>> -		clock-names = "xclk";
>>> -
>>> -		port {
>>> -			csi2_cam0: endpoint {
>>> -				remote-endpoint = <&csi2_phy0>;
>>> -				clock-lanes = <0>;
>>> -				data-lanes = <1 2>;
>>> -			};
>>> -		};
>>> -	};
>>> -
>>>   };
>>
>> As for patch 1/2, you could drop the two nodes completely. Same question
>> about overlay availability.
> 
> The &main_i2c1 node was added much before the OV5640 node in
> 19a1768fc34a (arm64: dts: ti: k3-am654-base-board: Add I2C nodes,
> 2018-11-13). I wonder if there is any reason for having it present even
> if there are no subnodes. One reason that I can think of is that this
> node defines the pinmux configuration and clock frequency which makes
> more sense here than in an overlay.

Right, and we also have an empty main_i2c2 there. I'd rather keep empty 
main_i2c1 to be in line with main_i2c2, and to have the pinmux in the 
main dts file. Unless someone can say we can remove both main_i2c1 and 
main_i2c2.

> 
>>
>>>   
>>>   &main_i2c2 {
>>> @@ -497,11 +475,6 @@ flash@0{
>>>   };
>>>   
>>>   &csi2_0 {
>>> -	csi2_phy0: endpoint {
>>> -		remote-endpoint = <&csi2_cam0>;
>>> -		clock-lanes = <0>;
>>> -		data-lanes = <1 2>;
>>> -	};
>>>   };
> 
> I agree with Laurent that the entire &csi2_0 node can be dropped.
> 
> Have you tested the CAL driver with this node removed and no overlay to
> add it back? Can it handle the error gracefully or does it crash and
> burn?

No, I haven't tested that for a while.

  Tomi

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] arm64: dts: ti: k3-am654-base-board: remove ov5640
@ 2021-04-12  9:15         ` Tomi Valkeinen
  0 siblings, 0 replies; 18+ messages in thread
From: Tomi Valkeinen @ 2021-04-12  9:15 UTC (permalink / raw)
  To: Pratyush Yadav, Laurent Pinchart
  Cc: Nishanth Menon, Benoit Parrot, devicetree, Rob Herring,
	linux-arm-kernel, Tony Lindgren, Vignesh Raghavendra

On 12/04/2021 11:36, Pratyush Yadav wrote:
> + Vignesh
> 
> On 12/04/21 11:00AM, Laurent Pinchart wrote:
>> Hi Tomi,
>>
>> Thank you for the patch.
>>
>> On Mon, Apr 12, 2021 at 10:53:06AM +0300, Tomi Valkeinen wrote:
>>> AM654 EVM boards are not shipped with OV5640 sensor module, it is a
>>> separate purchase. OV5640 module is also just one of the possible
>>> sensors or capture boards you can connect.
>>>
>>> However, for some reason, OV5640 has been added to the board dts file,
>>> making it cumbersome to use other sensors.
>>>
>>> Remove the OV5640 from the dts file so that it is easy to use other
>>> sensors via DT overlays.
>>>
>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>>> ---
>>>   .../arm64/boot/dts/ti/k3-am654-base-board.dts | 27 -------------------
>>>   1 file changed, 27 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
>>> index fe3043943906..76358b4944e1 100644
>>> --- a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
>>> +++ b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
>>> @@ -85,12 +85,6 @@ sw6 {
>>>   			gpios = <&wkup_gpio0 27 GPIO_ACTIVE_LOW>;
>>>   		};
>>>   	};
>>> -
>>> -	clk_ov5640_fixed: clock {
>>> -		compatible = "fixed-clock";
>>> -		#clock-cells = <0>;
>>> -		clock-frequency = <24000000>;
>>> -	};
>>>   };
>>>   
>>>   &wkup_pmx0 {
>>> @@ -288,22 +282,6 @@ &main_i2c1 {
>>>   	pinctrl-0 = <&main_i2c1_pins_default>;
>>>   	clock-frequency = <400000>;
>>>   
>>> -	ov5640: camera@3c {
>>> -		compatible = "ovti,ov5640";
>>> -		reg = <0x3c>;
>>> -
>>> -		clocks = <&clk_ov5640_fixed>;
>>> -		clock-names = "xclk";
>>> -
>>> -		port {
>>> -			csi2_cam0: endpoint {
>>> -				remote-endpoint = <&csi2_phy0>;
>>> -				clock-lanes = <0>;
>>> -				data-lanes = <1 2>;
>>> -			};
>>> -		};
>>> -	};
>>> -
>>>   };
>>
>> As for patch 1/2, you could drop the two nodes completely. Same question
>> about overlay availability.
> 
> The &main_i2c1 node was added much before the OV5640 node in
> 19a1768fc34a (arm64: dts: ti: k3-am654-base-board: Add I2C nodes,
> 2018-11-13). I wonder if there is any reason for having it present even
> if there are no subnodes. One reason that I can think of is that this
> node defines the pinmux configuration and clock frequency which makes
> more sense here than in an overlay.

Right, and we also have an empty main_i2c2 there. I'd rather keep empty 
main_i2c1 to be in line with main_i2c2, and to have the pinmux in the 
main dts file. Unless someone can say we can remove both main_i2c1 and 
main_i2c2.

> 
>>
>>>   
>>>   &main_i2c2 {
>>> @@ -497,11 +475,6 @@ flash@0{
>>>   };
>>>   
>>>   &csi2_0 {
>>> -	csi2_phy0: endpoint {
>>> -		remote-endpoint = <&csi2_cam0>;
>>> -		clock-lanes = <0>;
>>> -		data-lanes = <1 2>;
>>> -	};
>>>   };
> 
> I agree with Laurent that the entire &csi2_0 node can be dropped.
> 
> Have you tested the CAL driver with this node removed and no overlay to
> add it back? Can it handle the error gracefully or does it crash and
> burn?

No, I haven't tested that for a while.

  Tomi

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] arm64: dts: ti: k3-am654-base-board: remove ov5640
  2021-04-12  8:36       ` Pratyush Yadav
@ 2021-04-12  9:26         ` Vignesh Raghavendra
  -1 siblings, 0 replies; 18+ messages in thread
From: Vignesh Raghavendra @ 2021-04-12  9:26 UTC (permalink / raw)
  To: Yadav, Pratyush, Laurent Pinchart
  Cc: Tomi Valkeinen, Menon, Nishanth, Parrot, Benoit, devicetree,
	Rob Herring, linux-arm-kernel, Tony Lindgren



On 12/04/21 2:06 pm, Pratyush Yadav wrote:
> + Vignesh
> 
> On 12/04/21 11:00AM, Laurent Pinchart wrote:
>> Hi Tomi,
>>
>> Thank you for the patch.
>>
>> On Mon, Apr 12, 2021 at 10:53:06AM +0300, Tomi Valkeinen wrote:
>>> AM654 EVM boards are not shipped with OV5640 sensor module, it is a
>>> separate purchase. OV5640 module is also just one of the possible
>>> sensors or capture boards you can connect.
>>>
>>> However, for some reason, OV5640 has been added to the board dts file,
>>> making it cumbersome to use other sensors.
>>>
>>> Remove the OV5640 from the dts file so that it is easy to use other
>>> sensors via DT overlays.
>>>
>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>>> ---
>>>  .../arm64/boot/dts/ti/k3-am654-base-board.dts | 27 -------------------
>>>  1 file changed, 27 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
>>> index fe3043943906..76358b4944e1 100644
>>> --- a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
>>> +++ b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
>>> @@ -85,12 +85,6 @@ sw6 {
>>>                      gpios = <&wkup_gpio0 27 GPIO_ACTIVE_LOW>;
>>>              };
>>>      };
>>> -
>>> -   clk_ov5640_fixed: clock {
>>> -           compatible = "fixed-clock";
>>> -           #clock-cells = <0>;
>>> -           clock-frequency = <24000000>;
>>> -   };
>>>  };
>>>  
>>>  &wkup_pmx0 {
>>> @@ -288,22 +282,6 @@ &main_i2c1 {
>>>      pinctrl-0 = <&main_i2c1_pins_default>;
>>>      clock-frequency = <400000>;
>>>  
>>> -   ov5640: camera@3c {
>>> -           compatible = "ovti,ov5640";
>>> -           reg = <0x3c>;
>>> -
>>> -           clocks = <&clk_ov5640_fixed>;
>>> -           clock-names = "xclk";
>>> -
>>> -           port {
>>> -                   csi2_cam0: endpoint {
>>> -                           remote-endpoint = <&csi2_phy0>;
>>> -                           clock-lanes = <0>;
>>> -                           data-lanes = <1 2>;
>>> -                   };
>>> -           };
>>> -   };
>>> -
>>>  };
>>
>> As for patch 1/2, you could drop the two nodes completely. Same question
>> about overlay availability.
> 
> The &main_i2c1 node was added much before the OV5640 node in 
> 19a1768fc34a (arm64: dts: ti: k3-am654-base-board: Add I2C nodes, 
> 2018-11-13). I wonder if there is any reason for having it present even 
> if there are no subnodes. One reason that I can think of is that this 
> node defines the pinmux configuration and clock frequency which makes 
> more sense here than in an overlay.
> 

No, please don't drop main_i2c1 node. As long as pinmux is setup, its
possible to communicate with I2C devices from user space too even when
there are no subnodes.

-- 
Regards
Vignesh

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] arm64: dts: ti: k3-am654-base-board: remove ov5640
@ 2021-04-12  9:26         ` Vignesh Raghavendra
  0 siblings, 0 replies; 18+ messages in thread
From: Vignesh Raghavendra @ 2021-04-12  9:26 UTC (permalink / raw)
  To: Yadav, Pratyush, Laurent Pinchart
  Cc: Tomi Valkeinen, Menon, Nishanth, Parrot, Benoit, devicetree,
	Rob Herring, linux-arm-kernel, Tony Lindgren



On 12/04/21 2:06 pm, Pratyush Yadav wrote:
> + Vignesh
> 
> On 12/04/21 11:00AM, Laurent Pinchart wrote:
>> Hi Tomi,
>>
>> Thank you for the patch.
>>
>> On Mon, Apr 12, 2021 at 10:53:06AM +0300, Tomi Valkeinen wrote:
>>> AM654 EVM boards are not shipped with OV5640 sensor module, it is a
>>> separate purchase. OV5640 module is also just one of the possible
>>> sensors or capture boards you can connect.
>>>
>>> However, for some reason, OV5640 has been added to the board dts file,
>>> making it cumbersome to use other sensors.
>>>
>>> Remove the OV5640 from the dts file so that it is easy to use other
>>> sensors via DT overlays.
>>>
>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>>> ---
>>>  .../arm64/boot/dts/ti/k3-am654-base-board.dts | 27 -------------------
>>>  1 file changed, 27 deletions(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
>>> index fe3043943906..76358b4944e1 100644
>>> --- a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
>>> +++ b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
>>> @@ -85,12 +85,6 @@ sw6 {
>>>                      gpios = <&wkup_gpio0 27 GPIO_ACTIVE_LOW>;
>>>              };
>>>      };
>>> -
>>> -   clk_ov5640_fixed: clock {
>>> -           compatible = "fixed-clock";
>>> -           #clock-cells = <0>;
>>> -           clock-frequency = <24000000>;
>>> -   };
>>>  };
>>>  
>>>  &wkup_pmx0 {
>>> @@ -288,22 +282,6 @@ &main_i2c1 {
>>>      pinctrl-0 = <&main_i2c1_pins_default>;
>>>      clock-frequency = <400000>;
>>>  
>>> -   ov5640: camera@3c {
>>> -           compatible = "ovti,ov5640";
>>> -           reg = <0x3c>;
>>> -
>>> -           clocks = <&clk_ov5640_fixed>;
>>> -           clock-names = "xclk";
>>> -
>>> -           port {
>>> -                   csi2_cam0: endpoint {
>>> -                           remote-endpoint = <&csi2_phy0>;
>>> -                           clock-lanes = <0>;
>>> -                           data-lanes = <1 2>;
>>> -                   };
>>> -           };
>>> -   };
>>> -
>>>  };
>>
>> As for patch 1/2, you could drop the two nodes completely. Same question
>> about overlay availability.
> 
> The &main_i2c1 node was added much before the OV5640 node in 
> 19a1768fc34a (arm64: dts: ti: k3-am654-base-board: Add I2C nodes, 
> 2018-11-13). I wonder if there is any reason for having it present even 
> if there are no subnodes. One reason that I can think of is that this 
> node defines the pinmux configuration and clock frequency which makes 
> more sense here than in an overlay.
> 

No, please don't drop main_i2c1 node. As long as pinmux is setup, its
possible to communicate with I2C devices from user space too even when
there are no subnodes.

-- 
Regards
Vignesh

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] arm64: dts: ti: k3-am654-base-board: remove ov5640
  2021-04-12  7:53   ` Tomi Valkeinen
@ 2021-04-12 14:03     ` Nishanth Menon
  -1 siblings, 0 replies; 18+ messages in thread
From: Nishanth Menon @ 2021-04-12 14:03 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Benoit Parrot, Laurent Pinchart, devicetree, Rob Herring,
	linux-arm-kernel, Tony Lindgren, Pratyush Yadav

On 10:53-20210412, Tomi Valkeinen wrote:
> AM654 EVM boards are not shipped with OV5640 sensor module, it is a
> separate purchase. OV5640 module is also just one of the possible
> sensors or capture boards you can connect.
> 
> However, for some reason, OV5640 has been added to the board dts file,
> making it cumbersome to use other sensors.
> 
> Remove the OV5640 from the dts file so that it is easy to use other
> sensors via DT overlays.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  .../arm64/boot/dts/ti/k3-am654-base-board.dts | 27 -------------------
>  1 file changed, 27 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
> index fe3043943906..76358b4944e1 100644
> --- a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
> +++ b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts

will be good if you can submit this patch separately. dra7 and am654 are
maintained in different trees.


-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] arm64: dts: ti: k3-am654-base-board: remove ov5640
@ 2021-04-12 14:03     ` Nishanth Menon
  0 siblings, 0 replies; 18+ messages in thread
From: Nishanth Menon @ 2021-04-12 14:03 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Benoit Parrot, Laurent Pinchart, devicetree, Rob Herring,
	linux-arm-kernel, Tony Lindgren, Pratyush Yadav

On 10:53-20210412, Tomi Valkeinen wrote:
> AM654 EVM boards are not shipped with OV5640 sensor module, it is a
> separate purchase. OV5640 module is also just one of the possible
> sensors or capture boards you can connect.
> 
> However, for some reason, OV5640 has been added to the board dts file,
> making it cumbersome to use other sensors.
> 
> Remove the OV5640 from the dts file so that it is easy to use other
> sensors via DT overlays.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> ---
>  .../arm64/boot/dts/ti/k3-am654-base-board.dts | 27 -------------------
>  1 file changed, 27 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
> index fe3043943906..76358b4944e1 100644
> --- a/arch/arm64/boot/dts/ti/k3-am654-base-board.dts
> +++ b/arch/arm64/boot/dts/ti/k3-am654-base-board.dts

will be good if you can submit this patch separately. dra7 and am654 are
maintained in different trees.


-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2021-04-12 14:05 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-12  7:53 [PATCH 1/2] ARM: dts: dra76-evm: remove ov5640 Tomi Valkeinen
2021-04-12  7:53 ` Tomi Valkeinen
2021-04-12  7:53 ` [PATCH 2/2] arm64: dts: ti: k3-am654-base-board: " Tomi Valkeinen
2021-04-12  7:53   ` Tomi Valkeinen
2021-04-12  8:00   ` Laurent Pinchart
2021-04-12  8:00     ` Laurent Pinchart
2021-04-12  8:36     ` Pratyush Yadav
2021-04-12  8:36       ` Pratyush Yadav
2021-04-12  9:15       ` Tomi Valkeinen
2021-04-12  9:15         ` Tomi Valkeinen
2021-04-12  9:26       ` Vignesh Raghavendra
2021-04-12  9:26         ` Vignesh Raghavendra
2021-04-12 14:03   ` Nishanth Menon
2021-04-12 14:03     ` Nishanth Menon
2021-04-12  8:00 ` [PATCH 1/2] ARM: dts: dra76-evm: " Laurent Pinchart
2021-04-12  8:00   ` Laurent Pinchart
2021-04-12  8:26   ` Tomi Valkeinen
2021-04-12  8:26     ` Tomi Valkeinen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.