Re: [PATCH v3 4/5] arm64: dts: renesas: eagle: Add GMSL .dtsi

2021-04-15 Thread Jacopo Mondi
Hi Laurent,

On Thu, Apr 15, 2021 at 02:53:17AM +0300, Laurent Pinchart wrote:
> Hi Jacopo and Kieran,
>
> Thank you for the patch.
>
> On Wed, Apr 14, 2021 at 03:51:27PM +0200, Jacopo Mondi wrote:
> > From: Kieran Bingham 
> >
> > Describe the FAKRA connector available on Eagle board that allows
> > connecting GMSL camera modules such as IMI RDACM20 and RDACM21.
> >
> > Signed-off-by: Kieran Bingham 
> > Signed-off-by: Jacopo Mondi 
> > ---
> >  arch/arm64/boot/dts/renesas/eagle-gmsl.dtsi | 186 
> >  1 file changed, 186 insertions(+)
> >  create mode 100644 arch/arm64/boot/dts/renesas/eagle-gmsl.dtsi
> >
> > diff --git a/arch/arm64/boot/dts/renesas/eagle-gmsl.dtsi 
> > b/arch/arm64/boot/dts/renesas/eagle-gmsl.dtsi
> > new file mode 100644
> > index ..1836bca1e8b2
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/renesas/eagle-gmsl.dtsi
> > @@ -0,0 +1,186 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Device Tree Source (overlay) for the Eagle V3M GMSL connectors
> > + *
> > + * Copyright (C) 2017 Ideas on Board 
> > + * Copyright (C) 2021 Jacopo Mondi 
> > + *
> > + * This overlay allows you to define GMSL cameras connected to the FAKRA
> > + * connectors on the Eagle-V3M (or compatible) board.
> > + *
> > + * The following cameras are currently supported:
> > + *"imi,rdacm20"
> > + *"imi,rdacm21"
> > + */
> > +
> > +#include 
> > +
> > +/*
> > + * Select which cameras are in use:
> > + * #define EAGLE_CAMERA0_RDACM20
> > + * #define EAGLE_CAMERA0_RDACM21
> > + *
> > + * The two camera modules are configured with different image formats
> > + * and cannot be mixed.
> > + */
> > +#define EAGLE_CAMERA0_RDACM21
> > +#define EAGLE_CAMERA1_RDACM21
> > +#define EAGLE_CAMERA2_RDACM21
> > +#define EAGLE_CAMERA3_RDACM21
>
> To avoid possible errors, I'd use one macro to set the camera model, and
> another macro to select the ports. The second could be a bitmask if
> desired.
>
> These macros should be defined in the file that includes this file, not
> here.

Don't worry, I have quite some comments from you on the dts patches
from the previous versio of the series still to address... As said in
the cover letter I was mostly interested in a validation of the new
property and the driver support.

Thanks
  j

>
> > +
> > +/* Set the compatible string based on the camera model. */
> > +#if defined(EAGLE_CAMERA0_RDACM21) || defined(EAGLE_CAMERA1_RDACM21) || \
> > +defined(EAGLE_CAMERA2_RDACM21) || defined(EAGLE_CAMERA3_RDACM21)
> > +#define EAGLE_CAMERA_MODEL "imi,rdacm21"
> > +#define EAGLE_USE_RDACM21
> > +#elif defined(EAGLE_CAMERA0_RDACM20) || defined(EAGLE_CAMERA1_RDACM20) || \
> > +  defined(EAGLE_CAMERA2_RDACM20) || defined(EAGLE_CAMERA3_RDACM20)
> > +#define EAGLE_CAMERA_MODEL "imi,rdacm20"
> > +#define EAGLE_USE_RDACM20
> > +#endif
> > +
> > +/* Define which cameras are available. */
> > +#if defined(EAGLE_CAMERA0_RDACM21) || defined(EAGLE_CAMERA0_RDACM20)
> > +#define EAGLE_USE_CAMERA_0
> > +#endif
> > +
> > +#if defined(EAGLE_CAMERA1_RDACM21) || defined(EAGLE_CAMERA1_RDACM20)
> > +#define EAGLE_USE_CAMERA_1
> > +#endif
> > +
> > +#if defined(EAGLE_CAMERA2_RDACM21) || defined(EAGLE_CAMERA2_RDACM20)
> > +#define EAGLE_USE_CAMERA_2
> > +#endif
> > +
> > +#if defined(EAGLE_CAMERA3_RDACM21) || defined(EAGLE_CAMERA3_RDACM20)
> > +#define EAGLE_USE_CAMERA_3
> > +#endif
> > +
> > +/* Define the endpoint links. */
> > +#ifdef EAGLE_USE_CAMERA_0
> > +&max9286_in0 {
> > +   remote-endpoint = <&fakra_con0>;
> > +};
> > +#endif
> > +
> > +#ifdef EAGLE_USE_CAMERA_1
> > +&max9286_in1 {
> > +   remote-endpoint = <&fakra_con1>;
> > +};
> > +#endif
> > +
> > +#ifdef EAGLE_USE_CAMERA_2
> > +&max9286_in2 {
> > +   remote-endpoint = <&fakra_con2>;
> > +};
> > +#endif
> > +
> > +#ifdef EAGLE_USE_CAMERA_3
> > +&max9286_in3 {
> > +   remote-endpoint = <&fakra_con3>;
> > +};
> > +#endif
> > +
> > +/* Populate the GMSL i2c-mux bus with camera nodes. */
> > +#if defined(EAGLE_USE_RDACM21) || defined(EAGLE_USE_RDACM20)
> > +
> > +#ifdef EAGLE_USE_CAMERA_0
> > +&vin0 {
> > +   status = "okay";
> > +};
> > +#endif
> > +
> > +#ifdef EAGLE_USE_CAMERA_1
> > +&vin1 {
> > +   status = "okay";
> > +};
> > +#endif
> > +
> > +#ifdef EAGLE_USE_CAMERA_2
> > +&vin2 {
> > +   status = "okay";
> > +};
> > +#endif
> > +
> > +#ifdef EAGLE_USE_CAMERA_3
> > +&vin3 {
> > +   status = "okay";
> > +};
> > +#endif
> > +
> > +&gmsl {
> > +
> > +   status = "okay";
> > +   maxim,reverse-channel-microvolt = <10>;
> > +
> > +   i2c-mux {
> > +#ifdef EAGLE_USE_CAMERA_0
> > +   i2c@0 {
> > +   status = "okay";
> > +
> > +   camera@51 {
> > +   compatible = EAGLE_CAMERA_MODEL;
> > +   reg = <0x51>, <0x61>;
> > +
> > +   port {
> > +   fakra_con0: endpoint {
> > +   remote-endpoint = 
> > <&max9286_in0>;
> > +  

Re: [PATCH v3 4/5] arm64: dts: renesas: eagle: Add GMSL .dtsi

2021-04-14 Thread Laurent Pinchart
Hi Jacopo and Kieran,

Thank you for the patch.

On Wed, Apr 14, 2021 at 03:51:27PM +0200, Jacopo Mondi wrote:
> From: Kieran Bingham 
> 
> Describe the FAKRA connector available on Eagle board that allows
> connecting GMSL camera modules such as IMI RDACM20 and RDACM21.
> 
> Signed-off-by: Kieran Bingham 
> Signed-off-by: Jacopo Mondi 
> ---
>  arch/arm64/boot/dts/renesas/eagle-gmsl.dtsi | 186 
>  1 file changed, 186 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/renesas/eagle-gmsl.dtsi
> 
> diff --git a/arch/arm64/boot/dts/renesas/eagle-gmsl.dtsi 
> b/arch/arm64/boot/dts/renesas/eagle-gmsl.dtsi
> new file mode 100644
> index ..1836bca1e8b2
> --- /dev/null
> +++ b/arch/arm64/boot/dts/renesas/eagle-gmsl.dtsi
> @@ -0,0 +1,186 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Device Tree Source (overlay) for the Eagle V3M GMSL connectors
> + *
> + * Copyright (C) 2017 Ideas on Board 
> + * Copyright (C) 2021 Jacopo Mondi 
> + *
> + * This overlay allows you to define GMSL cameras connected to the FAKRA
> + * connectors on the Eagle-V3M (or compatible) board.
> + *
> + * The following cameras are currently supported:
> + *"imi,rdacm20"
> + *"imi,rdacm21"
> + */
> +
> +#include 
> +
> +/*
> + * Select which cameras are in use:
> + * #define EAGLE_CAMERA0_RDACM20
> + * #define EAGLE_CAMERA0_RDACM21
> + *
> + * The two camera modules are configured with different image formats
> + * and cannot be mixed.
> + */
> +#define EAGLE_CAMERA0_RDACM21
> +#define EAGLE_CAMERA1_RDACM21
> +#define EAGLE_CAMERA2_RDACM21
> +#define EAGLE_CAMERA3_RDACM21

To avoid possible errors, I'd use one macro to set the camera model, and
another macro to select the ports. The second could be a bitmask if
desired.

These macros should be defined in the file that includes this file, not
here.

> +
> +/* Set the compatible string based on the camera model. */
> +#if defined(EAGLE_CAMERA0_RDACM21) || defined(EAGLE_CAMERA1_RDACM21) || \
> +defined(EAGLE_CAMERA2_RDACM21) || defined(EAGLE_CAMERA3_RDACM21)
> +#define EAGLE_CAMERA_MODEL   "imi,rdacm21"
> +#define EAGLE_USE_RDACM21
> +#elif defined(EAGLE_CAMERA0_RDACM20) || defined(EAGLE_CAMERA1_RDACM20) || \
> +  defined(EAGLE_CAMERA2_RDACM20) || defined(EAGLE_CAMERA3_RDACM20)
> +#define EAGLE_CAMERA_MODEL   "imi,rdacm20"
> +#define EAGLE_USE_RDACM20
> +#endif
> +
> +/* Define which cameras are available. */
> +#if defined(EAGLE_CAMERA0_RDACM21) || defined(EAGLE_CAMERA0_RDACM20)
> +#define EAGLE_USE_CAMERA_0
> +#endif
> +
> +#if defined(EAGLE_CAMERA1_RDACM21) || defined(EAGLE_CAMERA1_RDACM20)
> +#define EAGLE_USE_CAMERA_1
> +#endif
> +
> +#if defined(EAGLE_CAMERA2_RDACM21) || defined(EAGLE_CAMERA2_RDACM20)
> +#define EAGLE_USE_CAMERA_2
> +#endif
> +
> +#if defined(EAGLE_CAMERA3_RDACM21) || defined(EAGLE_CAMERA3_RDACM20)
> +#define EAGLE_USE_CAMERA_3
> +#endif
> +
> +/* Define the endpoint links. */
> +#ifdef EAGLE_USE_CAMERA_0
> +&max9286_in0 {
> + remote-endpoint = <&fakra_con0>;
> +};
> +#endif
> +
> +#ifdef EAGLE_USE_CAMERA_1
> +&max9286_in1 {
> + remote-endpoint = <&fakra_con1>;
> +};
> +#endif
> +
> +#ifdef EAGLE_USE_CAMERA_2
> +&max9286_in2 {
> + remote-endpoint = <&fakra_con2>;
> +};
> +#endif
> +
> +#ifdef EAGLE_USE_CAMERA_3
> +&max9286_in3 {
> + remote-endpoint = <&fakra_con3>;
> +};
> +#endif
> +
> +/* Populate the GMSL i2c-mux bus with camera nodes. */
> +#if defined(EAGLE_USE_RDACM21) || defined(EAGLE_USE_RDACM20)
> +
> +#ifdef EAGLE_USE_CAMERA_0
> +&vin0 {
> + status = "okay";
> +};
> +#endif
> +
> +#ifdef EAGLE_USE_CAMERA_1
> +&vin1 {
> + status = "okay";
> +};
> +#endif
> +
> +#ifdef EAGLE_USE_CAMERA_2
> +&vin2 {
> + status = "okay";
> +};
> +#endif
> +
> +#ifdef EAGLE_USE_CAMERA_3
> +&vin3 {
> + status = "okay";
> +};
> +#endif
> +
> +&gmsl {
> +
> + status = "okay";
> + maxim,reverse-channel-microvolt = <10>;
> +
> + i2c-mux {
> +#ifdef EAGLE_USE_CAMERA_0
> + i2c@0 {
> + status = "okay";
> +
> + camera@51 {
> + compatible = EAGLE_CAMERA_MODEL;
> + reg = <0x51>, <0x61>;
> +
> + port {
> + fakra_con0: endpoint {
> + remote-endpoint = 
> <&max9286_in0>;
> + };
> + };
> + };
> + };
> +#endif
> +
> +#ifdef EAGLE_USE_CAMERA_1
> + i2c@1 {
> + status = "okay";
> +
> + camera@52 {
> + compatible = EAGLE_CAMERA_MODEL;
> + reg = <0x52>, <0x62>;
> +
> + port {
> + fakra_con1: endpoint {
> + remote-endpoint = 
> <&max9286_in1>;
> +

[PATCH v3 4/5] arm64: dts: renesas: eagle: Add GMSL .dtsi

2021-04-14 Thread Jacopo Mondi
From: Kieran Bingham 

Describe the FAKRA connector available on Eagle board that allows
connecting GMSL camera modules such as IMI RDACM20 and RDACM21.

Signed-off-by: Kieran Bingham 
Signed-off-by: Jacopo Mondi 
---
 arch/arm64/boot/dts/renesas/eagle-gmsl.dtsi | 186 
 1 file changed, 186 insertions(+)
 create mode 100644 arch/arm64/boot/dts/renesas/eagle-gmsl.dtsi

diff --git a/arch/arm64/boot/dts/renesas/eagle-gmsl.dtsi 
b/arch/arm64/boot/dts/renesas/eagle-gmsl.dtsi
new file mode 100644
index ..1836bca1e8b2
--- /dev/null
+++ b/arch/arm64/boot/dts/renesas/eagle-gmsl.dtsi
@@ -0,0 +1,186 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Device Tree Source (overlay) for the Eagle V3M GMSL connectors
+ *
+ * Copyright (C) 2017 Ideas on Board 
+ * Copyright (C) 2021 Jacopo Mondi 
+ *
+ * This overlay allows you to define GMSL cameras connected to the FAKRA
+ * connectors on the Eagle-V3M (or compatible) board.
+ *
+ * The following cameras are currently supported:
+ *"imi,rdacm20"
+ *"imi,rdacm21"
+ */
+
+#include 
+
+/*
+ * Select which cameras are in use:
+ * #define EAGLE_CAMERA0_RDACM20
+ * #define EAGLE_CAMERA0_RDACM21
+ *
+ * The two camera modules are configured with different image formats
+ * and cannot be mixed.
+ */
+#define EAGLE_CAMERA0_RDACM21
+#define EAGLE_CAMERA1_RDACM21
+#define EAGLE_CAMERA2_RDACM21
+#define EAGLE_CAMERA3_RDACM21
+
+/* Set the compatible string based on the camera model. */
+#if defined(EAGLE_CAMERA0_RDACM21) || defined(EAGLE_CAMERA1_RDACM21) || \
+defined(EAGLE_CAMERA2_RDACM21) || defined(EAGLE_CAMERA3_RDACM21)
+#define EAGLE_CAMERA_MODEL "imi,rdacm21"
+#define EAGLE_USE_RDACM21
+#elif defined(EAGLE_CAMERA0_RDACM20) || defined(EAGLE_CAMERA1_RDACM20) || \
+  defined(EAGLE_CAMERA2_RDACM20) || defined(EAGLE_CAMERA3_RDACM20)
+#define EAGLE_CAMERA_MODEL "imi,rdacm20"
+#define EAGLE_USE_RDACM20
+#endif
+
+/* Define which cameras are available. */
+#if defined(EAGLE_CAMERA0_RDACM21) || defined(EAGLE_CAMERA0_RDACM20)
+#define EAGLE_USE_CAMERA_0
+#endif
+
+#if defined(EAGLE_CAMERA1_RDACM21) || defined(EAGLE_CAMERA1_RDACM20)
+#define EAGLE_USE_CAMERA_1
+#endif
+
+#if defined(EAGLE_CAMERA2_RDACM21) || defined(EAGLE_CAMERA2_RDACM20)
+#define EAGLE_USE_CAMERA_2
+#endif
+
+#if defined(EAGLE_CAMERA3_RDACM21) || defined(EAGLE_CAMERA3_RDACM20)
+#define EAGLE_USE_CAMERA_3
+#endif
+
+/* Define the endpoint links. */
+#ifdef EAGLE_USE_CAMERA_0
+&max9286_in0 {
+   remote-endpoint = <&fakra_con0>;
+};
+#endif
+
+#ifdef EAGLE_USE_CAMERA_1
+&max9286_in1 {
+   remote-endpoint = <&fakra_con1>;
+};
+#endif
+
+#ifdef EAGLE_USE_CAMERA_2
+&max9286_in2 {
+   remote-endpoint = <&fakra_con2>;
+};
+#endif
+
+#ifdef EAGLE_USE_CAMERA_3
+&max9286_in3 {
+   remote-endpoint = <&fakra_con3>;
+};
+#endif
+
+/* Populate the GMSL i2c-mux bus with camera nodes. */
+#if defined(EAGLE_USE_RDACM21) || defined(EAGLE_USE_RDACM20)
+
+#ifdef EAGLE_USE_CAMERA_0
+&vin0 {
+   status = "okay";
+};
+#endif
+
+#ifdef EAGLE_USE_CAMERA_1
+&vin1 {
+   status = "okay";
+};
+#endif
+
+#ifdef EAGLE_USE_CAMERA_2
+&vin2 {
+   status = "okay";
+};
+#endif
+
+#ifdef EAGLE_USE_CAMERA_3
+&vin3 {
+   status = "okay";
+};
+#endif
+
+&gmsl {
+
+   status = "okay";
+   maxim,reverse-channel-microvolt = <10>;
+
+   i2c-mux {
+#ifdef EAGLE_USE_CAMERA_0
+   i2c@0 {
+   status = "okay";
+
+   camera@51 {
+   compatible = EAGLE_CAMERA_MODEL;
+   reg = <0x51>, <0x61>;
+
+   port {
+   fakra_con0: endpoint {
+   remote-endpoint = 
<&max9286_in0>;
+   };
+   };
+   };
+   };
+#endif
+
+#ifdef EAGLE_USE_CAMERA_1
+   i2c@1 {
+   status = "okay";
+
+   camera@52 {
+   compatible = EAGLE_CAMERA_MODEL;
+   reg = <0x52>, <0x62>;
+
+   port {
+   fakra_con1: endpoint {
+   remote-endpoint = 
<&max9286_in1>;
+   };
+   };
+   };
+   };
+#endif
+
+#ifdef EAGLE_USE_CAMERA_2
+   i2c@2 {
+   status = "okay";
+
+   camera@53 {
+   compatible = EAGLE_CAMERA_MODEL;
+   reg = <0x53>, <0x63>;
+
+   port {
+   fakra_con2: endpoint {
+   remote-endpoint = 
<&max9286_in2>;
+   };
+