Re: [PATCH v1 01/12] arm64: boot: dts: salvator-x: Add camera module

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Jacopo,

Thank you for the patch.

On Monday 19 Jun 2017 19:04:38 Jacopo Mondi wrote:
> Add camera module to Salvator-X M3W device tree.
> The camera module sits on a i2c-gpio interface and it connected to VIN
> channel #4 parallel video input port.
> 
> Signed-off-by: Jacopo Mondi <jacopo+renesas@jmondi.org>
> ---
>  arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts | 35 +++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
> b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts index ca4eb98..41c94c3
> 100644
> --- a/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
> +++ b/arch/arm64/boot/dts/renesas/r8a7796-salvator-x.dts
> @@ -103,6 +103,41 @@
>  		states = <3300000 1
>  			  1800000 0>;
>  	};
> +
> +	camera_module {

This node describes an I2C master controller, it should be called accordingly.

> +		compatible = "i2c-gpio";
> +		gpios = <&gpio1 9 GPIO_ACTIVE_HIGH
> +			 &gpio1 11 GPIO_ACTIVE_HIGH
> +			>;
> +
> +		i2c-gpio,delay-us = <4>;
> +
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		mclk: xclk {
> +			compatible = "fixed-clock";
> +			#clock-cells = <1>;
> +			clock-frequency  = <27000000>;
> +			clock-output-names = "mclk";
> +		};

This node describes a fixed oscillator not connected to the I2C bus, so it 
should be moved to the top-level of the device tree instead of being a child 
of the I2C controller.

> +		camera: mt9m111@48 {

No need for a label, and you should call the node according to its function, 
not the device model. Maybe something like camera-sensor@48 ? Or just 
camera@48 ?

> +			compatible = "micron,mt9m111";
> +			reg = <0x48>;
> +
> +			clocks = <&mclk 0>;
> +			clock-names = "mclk";
> +
> +			port {
> +				mt9m111_out: endpoint {
> +					bus_width = <8>;
> +					remote-endpoint = <&vin4_in>;
> +				};
> +			};
> +		};
> +	};
> +
>  };
> 
>  &pfc {

-- 
Regards,

Laurent Pinchart