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

[Adrien Grassein]

Hi Shawn,

Thanks for your review.

Le mar. 11 mai 2021 à 04:27, Shawn Guo <shawnguo at 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 at 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 at 68 {
> >       };
> >  };
> >
> > +&i2c4 {
> > +     clock-frequency = <100000>;
> > +     pinctrl-names = "default";
> > +     pinctrl-0 = <&pinctrl_i2c4>;
> > +     status = "okay";
> > +
> > +     pca9546: i2cmux9546 at 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 at 0 {
>
> Is number 4 really needed in node name?
>
I add 4 to keep it coherent with the DTS file (i2c1 at 0 for example).

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