diff mbox series

[v3,3/4] arm64: dts: imx8mq-nitrogen: add lt8912 MIPI-DSI to HDMI

Message ID 20210331232356.2204476-4-adrien.grassein@gmail.com (mailing list archive)
State New, archived
Headers show
Series Add peripheral support to imx8mq-nitrogen board | expand

Commit Message

Adrien Grassein March 31, 2021, 11:23 p.m. UTC 
Add support of the lt8912b in the DTB.
This adds the support of the DB_DSIHD daugther board from
Boundary Devices.

Signed-off-by: Adrien Grassein <adrien.grassein@gmail.com>
---
 .../boot/dts/freescale/imx8mq-nitrogen.dts    | 120 ++++++++++++++++++
 1 file changed, 120 insertions(+)

Comments

Shawn Guo May 11, 2021, 2:27 a.m. UTC | #1 
On Thu, Apr 01, 2021 at 01:23:55AM +0200, Adrien Grassein wrote:
> Add support of the lt8912b in the DTB.
> This adds the support of the DB_DSIHD daugther board from
> Boundary Devices.
> 
> Signed-off-by: Adrien Grassein <adrien.grassein@gmail.com>
> ---
>  .../boot/dts/freescale/imx8mq-nitrogen.dts    | 120 ++++++++++++++++++
>  1 file changed, 120 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mq-nitrogen.dts b/arch/arm64/boot/dts/freescale/imx8mq-nitrogen.dts
> index 04992cbba56e..4ffd23ea705f 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mq-nitrogen.dts
> +++ b/arch/arm64/boot/dts/freescale/imx8mq-nitrogen.dts
> @@ -34,6 +34,19 @@ power {
>  		};
>  	};
>  
> +	hdmi-connector {
> +		compatible = "hdmi-connector";
> +		ddc-i2c-bus = <&ddc_i2c_bus>;
> +		label = "hdmi";
> +		type = "a";
> +
> +		port {
> +			hdmi_connector_in: endpoint {
> +				remote-endpoint = <&lt8912_out>;
> +			};
> +		};
> +	};
> +
>  	reg_usb_otg_vbus: regulator-usb-otg-vbus {
>  		compatible = "regulator-fixed";
>  		pinctrl-names = "default";
> @@ -81,6 +94,9 @@ reg_vref_5v: regulator-vref-5v {
>  	};
>  };
>  
> +&dphy {
> +	status = "okay";
> +};
>  
>  &fec1 {
>  	pinctrl-names = "default";
> @@ -193,6 +209,97 @@ rtc@68 {
>  	};
>  };
>  
> +&i2c4 {
> +	clock-frequency = <100000>;
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_i2c4>;
> +	status = "okay";
> +
> +	pca9546: i2cmux9546@70 {

Node name should be generic, so 9546 should be dropped from there?

> +		compatible = "nxp,pca9546";
> +		reg = <0x70>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		i2c4@0 {

Is number 4 really needed in node name?

> +			reg = <0>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			clock-frequency = <100000>;
> +
> +			hdmi-bridge@48 {
> +				compatible = "lontium,lt8912b";
> +				reg = <0x48> ;
> +				reset-gpios = <&max7323 0 GPIO_ACTIVE_LOW>;
> +
> +				ports {
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +
> +					port@0 {
> +						reg = <0>;
> +
> +						hdmi_out_in: endpoint {
> +							data-lanes = <1 2 3 4>;
> +							remote-endpoint = <&mipi_dsi_out>;
> +						};
> +					};
> +
> +					port@1 {
> +						reg = <1>;
> +
> +						lt8912_out: endpoint {
> +							remote-endpoint = <&hdmi_connector_in>;
> +						};
> +					};
> +				};
> +			};
> +		};
> +
> +		ddc_i2c_bus: i2c4@1 {
> +			reg = <1>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			clock-frequency = <100000>;
> +		};
> +
> +		i2c4@3 {
> +			reg = <3>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			clock-frequency = <100000>;
> +
> +			max7323: max7323@68 {

Can we have a generic node name for this device?

> +				compatible = "maxim,max7323";
> +				pinctrl-names = "default";
> +				pinctrl-0 = <&pinctrl_max7323>;
> +				gpio-controller;
> +				reg = <0x68>;
> +				#gpio-cells = <2>;
> +			};
> +		};
> +	};
> +};
> +
> +&lcdif {
> +	status = "okay";
> +};
> +
> +&mipi_dsi {
> +	status = "okay";

Move it to end of property list.

> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +
> +	ports {
> +		port@1 {
> +			reg = <1>;

Newline between property and child node.

Shawn

> +			mipi_dsi_out: endpoint {
> +				remote-endpoint = <&hdmi_out_in>;
> +			};
> +		};
> +	};
> +};
> +
>  &uart1 { /* console */
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_uart1>;
> @@ -368,6 +475,19 @@ MX8MQ_IOMUXC_GPIO1_IO06_GPIO1_IO6		0x49
>  		>;
>  	};
>  
> +	pinctrl_i2c4: i2c4grp {
> +		fsl,pins = <
> +			MX8MQ_IOMUXC_I2C4_SCL_I2C4_SCL			0x4000007f
> +			MX8MQ_IOMUXC_I2C4_SDA_I2C4_SDA			0x4000007f
> +		>;
> +	};
> +
> +	pinctrl_max7323: max7323grp {
> +		fsl,pins = <
> +			MX8MQ_IOMUXC_NAND_RE_B_GPIO3_IO15 0x19
> +		>;
> +	};
> +
>  	pinctrl_reg_arm_dram: reg-arm-dramgrp {
>  		fsl,pins = <
>  			MX8MQ_IOMUXC_SAI5_RXD3_GPIO3_IO24	0x16
> -- 
> 2.25.1
>
Adrien Grassein May 11, 2021, 7:36 p.m. UTC | #2 
Hi Shawn,

Thanks for your review.

Le mar. 11 mai 2021 à 04:27, Shawn Guo <shawnguo@kernel.org> a écrit :
>
> On Thu, Apr 01, 2021 at 01:23:55AM +0200, Adrien Grassein wrote:
> > Add support of the lt8912b in the DTB.
> > This adds the support of the DB_DSIHD daugther board from
> > Boundary Devices.
> >
> > Signed-off-by: Adrien Grassein <adrien.grassein@gmail.com>
> > ---
> >  .../boot/dts/freescale/imx8mq-nitrogen.dts    | 120 ++++++++++++++++++
> >  1 file changed, 120 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mq-nitrogen.dts b/arch/arm64/boot/dts/freescale/imx8mq-nitrogen.dts
> > index 04992cbba56e..4ffd23ea705f 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mq-nitrogen.dts
> > +++ b/arch/arm64/boot/dts/freescale/imx8mq-nitrogen.dts
> > @@ -34,6 +34,19 @@ power {
> >               };
> >       };
> >
> > +     hdmi-connector {
> > +             compatible = "hdmi-connector";
> > +             ddc-i2c-bus = <&ddc_i2c_bus>;
> > +             label = "hdmi";
> > +             type = "a";
> > +
> > +             port {
> > +                     hdmi_connector_in: endpoint {
> > +                             remote-endpoint = <&lt8912_out>;
> > +                     };
> > +             };
> > +     };
> > +
> >       reg_usb_otg_vbus: regulator-usb-otg-vbus {
> >               compatible = "regulator-fixed";
> >               pinctrl-names = "default";
> > @@ -81,6 +94,9 @@ reg_vref_5v: regulator-vref-5v {
> >       };
> >  };
> >
> > +&dphy {
> > +     status = "okay";
> > +};
> >
> >  &fec1 {
> >       pinctrl-names = "default";
> > @@ -193,6 +209,97 @@ rtc@68 {
> >       };
> >  };
> >
> > +&i2c4 {
> > +     clock-frequency = <100000>;
> > +     pinctrl-names = "default";
> > +     pinctrl-0 = <&pinctrl_i2c4>;
> > +     status = "okay";
> > +
> > +     pca9546: i2cmux9546@70 {
>
> Node name should be generic, so 9546 should be dropped from there?
>
> > +             compatible = "nxp,pca9546";
> > +             reg = <0x70>;
> > +             #address-cells = <1>;
> > +             #size-cells = <0>;
> > +
> > +             i2c4@0 {
>
> Is number 4 really needed in node name?
>
I add 4 to keep it coherent with the DTS file (i2c1@0 for example).

> > +                     reg = <0>;
> > +                     #address-cells = <1>;
> > +                     #size-cells = <0>;
> > +                     clock-frequency = <100000>;
> > +
> > +                     hdmi-bridge@48 {
> > +                             compatible = "lontium,lt8912b";
> > +                             reg = <0x48> ;
> > +                             reset-gpios = <&max7323 0 GPIO_ACTIVE_LOW>;
> > +
> > +                             ports {
> > +                                     #address-cells = <1>;
> > +                                     #size-cells = <0>;
> > +
> > +                                     port@0 {
> > +                                             reg = <0>;
> > +
> > +                                             hdmi_out_in: endpoint {
> > +                                                     data-lanes = <1 2 3 4>;
> > +                                                     remote-endpoint = <&mipi_dsi_out>;
> > +                                             };
> > +                                     };
> > +
> > +                                     port@1 {
> > +                                             reg = <1>;
> > +
> > +                                             lt8912_out: endpoint {
> > +                                                     remote-endpoint = <&hdmi_connector_in>;
> > +                                             };
> > +                                     };
> > +                             };
> > +                     };
> > +             };
> > +
> > +             ddc_i2c_bus: i2c4@1 {
> > +                     reg = <1>;
> > +                     #address-cells = <1>;
> > +                     #size-cells = <0>;
> > +                     clock-frequency = <100000>;
> > +             };
> > +
> > +             i2c4@3 {
> > +                     reg = <3>;
> > +                     #address-cells = <1>;
> > +                     #size-cells = <0>;
> > +                     clock-frequency = <100000>;
> > +
> > +                     max7323: max7323@68 {
>
> Can we have a generic node name for this device?
>
> > +                             compatible = "maxim,max7323";
> > +                             pinctrl-names = "default";
> > +                             pinctrl-0 = <&pinctrl_max7323>;
> > +                             gpio-controller;
> > +                             reg = <0x68>;
> > +                             #gpio-cells = <2>;
> > +                     };
> > +             };
> > +     };
> > +};
> > +
> > +&lcdif {
> > +     status = "okay";
> > +};
> > +
> > +&mipi_dsi {
> > +     status = "okay";
>
> Move it to end of property list.
>
> > +     #address-cells = <1>;
> > +     #size-cells = <0>;
> > +
> > +     ports {
> > +             port@1 {
> > +                     reg = <1>;
>
> Newline between property and child node.
>
> Shawn
>
> > +                     mipi_dsi_out: endpoint {
> > +                             remote-endpoint = <&hdmi_out_in>;
> > +                     };
> > +             };
> > +     };
> > +};
> > +
> >  &uart1 { /* console */
> >       pinctrl-names = "default";
> >       pinctrl-0 = <&pinctrl_uart1>;
> > @@ -368,6 +475,19 @@ MX8MQ_IOMUXC_GPIO1_IO06_GPIO1_IO6                0x49
> >               >;
> >       };
> >
> > +     pinctrl_i2c4: i2c4grp {
> > +             fsl,pins = <
> > +                     MX8MQ_IOMUXC_I2C4_SCL_I2C4_SCL                  0x4000007f
> > +                     MX8MQ_IOMUXC_I2C4_SDA_I2C4_SDA                  0x4000007f
> > +             >;
> > +     };
> > +
> > +     pinctrl_max7323: max7323grp {
> > +             fsl,pins = <
> > +                     MX8MQ_IOMUXC_NAND_RE_B_GPIO3_IO15 0x19
> > +             >;
> > +     };
> > +
> >       pinctrl_reg_arm_dram: reg-arm-dramgrp {
> >               fsl,pins = <
> >                       MX8MQ_IOMUXC_SAI5_RXD3_GPIO3_IO24       0x16
> > --
> > 2.25.1
> >

The other comments are OK.
I will send you an updated patch set.

Thanks again,
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/freescale/imx8mq-nitrogen.dts b/arch/arm64/boot/dts/freescale/imx8mq-nitrogen.dts
index 04992cbba56e..4ffd23ea705f 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq-nitrogen.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mq-nitrogen.dts
@@ -34,6 +34,19 @@  power {
 		};
 	};
 
+	hdmi-connector {
+		compatible = "hdmi-connector";
+		ddc-i2c-bus = <&ddc_i2c_bus>;
+		label = "hdmi";
+		type = "a";
+
+		port {
+			hdmi_connector_in: endpoint {
+				remote-endpoint = <&lt8912_out>;
+			};
+		};
+	};
+
 	reg_usb_otg_vbus: regulator-usb-otg-vbus {
 		compatible = "regulator-fixed";
 		pinctrl-names = "default";
@@ -81,6 +94,9 @@  reg_vref_5v: regulator-vref-5v {
 	};
 };
 
+&dphy {
+	status = "okay";
+};
 
 &fec1 {
 	pinctrl-names = "default";
@@ -193,6 +209,97 @@  rtc@68 {
 	};
 };
 
+&i2c4 {
+	clock-frequency = <100000>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&pinctrl_i2c4>;
+	status = "okay";
+
+	pca9546: i2cmux9546@70 {
+		compatible = "nxp,pca9546";
+		reg = <0x70>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		i2c4@0 {
+			reg = <0>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			clock-frequency = <100000>;
+
+			hdmi-bridge@48 {
+				compatible = "lontium,lt8912b";
+				reg = <0x48> ;
+				reset-gpios = <&max7323 0 GPIO_ACTIVE_LOW>;
+
+				ports {
+					#address-cells = <1>;
+					#size-cells = <0>;
+
+					port@0 {
+						reg = <0>;
+
+						hdmi_out_in: endpoint {
+							data-lanes = <1 2 3 4>;
+							remote-endpoint = <&mipi_dsi_out>;
+						};
+					};
+
+					port@1 {
+						reg = <1>;
+
+						lt8912_out: endpoint {
+							remote-endpoint = <&hdmi_connector_in>;
+						};
+					};
+				};
+			};
+		};
+
+		ddc_i2c_bus: i2c4@1 {
+			reg = <1>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			clock-frequency = <100000>;
+		};
+
+		i2c4@3 {
+			reg = <3>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			clock-frequency = <100000>;
+
+			max7323: max7323@68 {
+				compatible = "maxim,max7323";
+				pinctrl-names = "default";
+				pinctrl-0 = <&pinctrl_max7323>;
+				gpio-controller;
+				reg = <0x68>;
+				#gpio-cells = <2>;
+			};
+		};
+	};
+};
+
+&lcdif {
+	status = "okay";
+};
+
+&mipi_dsi {
+	status = "okay";
+	#address-cells = <1>;
+	#size-cells = <0>;
+
+	ports {
+		port@1 {
+			reg = <1>;
+			mipi_dsi_out: endpoint {
+				remote-endpoint = <&hdmi_out_in>;
+			};
+		};
+	};
+};
+
 &uart1 { /* console */
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_uart1>;
@@ -368,6 +475,19 @@  MX8MQ_IOMUXC_GPIO1_IO06_GPIO1_IO6		0x49
 		>;
 	};
 
+	pinctrl_i2c4: i2c4grp {
+		fsl,pins = <
+			MX8MQ_IOMUXC_I2C4_SCL_I2C4_SCL			0x4000007f
+			MX8MQ_IOMUXC_I2C4_SDA_I2C4_SDA			0x4000007f
+		>;
+	};
+
+	pinctrl_max7323: max7323grp {
+		fsl,pins = <
+			MX8MQ_IOMUXC_NAND_RE_B_GPIO3_IO15 0x19
+		>;
+	};
+
 	pinctrl_reg_arm_dram: reg-arm-dramgrp {
 		fsl,pins = <
 			MX8MQ_IOMUXC_SAI5_RXD3_GPIO3_IO24	0x16