[PATCH 2/2] arm64: dts: ti: k3-am654-base-board: remove ov5640

[Tomi Valkeinen]

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 at 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 at 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 at 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