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@xxxxxxxxxxxxxxxx>
>>> ---
>>> .../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
