Received-SPF: pass (google.com: domain of linux-kernel+bounces-65083-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Wed, 14 Feb 2024 11:52:23 +0100
From: Francesco Dolcini <francesco@dolcini.it>
To: Roland Hieber <rhi@pengutronix.de>,
	Linux Regressions <regressions@lists.linux.dev>
Cc: Hiago De Franco <hiagofranco@gmail.com>,
	Marco Felsch <m.felsch@pengutronix.de>,
	Shawn Guo <shawnguo@kernel.org>, Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Philipp Zabel <p.zabel@pengutronix.de>, kernel@pengutronix.de,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: MXSFB error: -ENODEV: Cannot connect bridge
Message-ID: <20240214105223.GA78582@francesco-nb>
References: <34yzygh3mbwpqr2re7nxmhyxy3s7qmqy4vhxvoyxnoguktriur@z66m7gvpqlia>
 <20240212110706.ibrreoj2wgzhltyw@pengutronix.de>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240212110706.ibrreoj2wgzhltyw@pengutronix.de>

+ Linux regression.

This is a regression on v6.8-rc1.

On Mon, Feb 12, 2024 at 12:07:06PM +0100, Roland Hieber wrote:
> On Thu, Feb 08, 2024 at 12:58:02PM -0300, Hiago De Franco wrote:
> > Hello all,
> > 
> > while doing some tests with kernel v6.8-rc3 and Colibri iMX7D, we
> > noticed the following error:
> > 
> > [    0.432547] mxsfb 30730000.lcdif: error -ENODEV: Cannot connect bridge
> > 
> > This was introduced by commit edbbae7fba495284f72f05768696572691231558
> > ("ARM: dts: imx7: add MIPI-DSI support"). This patch is routing the
> > lcdif to the mipi_dsi_in_lcdif endpoint, however we do not have the DSI
> > pins available in our edge connector. Instead, we use the parallel RGB
> > LCD interface directly with, as example, an external LVDS transmitter:
> > 
> > &lcdif {
> > ...
> > 	status = "disabled";
> > 
> > 	port {
> > 		lcdif_out: endpoint {
> > 			remote-endpoint = <&lcd_panel_in>;
> > 		};
> > 	};
> > };
> > 
> > By applying the following patch, the issue is gone and the LVDS works
> > again:
> > 
> > diff --git a/arch/arm/boot/dts/nxp/imx/imx7s.dtsi b/arch/arm/boot/dts/nxp/imx/imx7s.dtsi
> > index ebf7befcc11e..9c81c6baa2d3 100644
> > --- a/arch/arm/boot/dts/nxp/imx/imx7s.dtsi
> > +++ b/arch/arm/boot/dts/nxp/imx/imx7s.dtsi
> > @@ -834,16 +834,6 @@ lcdif: lcdif@30730000 {
> >  					<&clks IMX7D_LCDIF_PIXEL_ROOT_CLK>;
> >  				clock-names = "pix", "axi";
> >  				status = "disabled";
> > -
> > -				port {
> > -					#address-cells = <1>;
> > -					#size-cells = <0>;
> > -
> > -					lcdif_out_mipi_dsi: endpoint@0 {
> > -						reg = <0>;
> > -						remote-endpoint = <&mipi_dsi_in_lcdif>;
> > -					};
> > -				};
> >  			};
> >  
> >  			mipi_csi: mipi-csi@30750000 {
> > @@ -895,22 +885,6 @@ mipi_dsi: dsi@30760000 {
> >  				samsung,esc-clock-frequency = <20000000>;
> >  				samsung,pll-clock-frequency = <24000000>;
> >  				status = "disabled";
> > -
> > -				ports {
> > -					#address-cells = <1>;
> > -					#size-cells = <0>;
> > -
> > -					port@0 {
> > -						reg = <0>;
> > -						#address-cells = <1>;
> > -						#size-cells = <0>;
> > -
> > -						mipi_dsi_in_lcdif: endpoint@0 {
> > -							reg = <0>;
> > -							remote-endpoint = <&lcdif_out_mipi_dsi>;
> > -						};
> > -					};
> > -				};
> >  			};
> >  		};
> > 
> > I would like to know your opinion about this patch before sending it,
> > does it makes sense for you? I understand that routing to endpoint
> > should be done in the SoM device tree, so we are free to rout other
> > endpoint without issues.
> 
> As far as I understood, the LCDIF -> DSI connection is always present in
> the SoC. Can you overwrite the routing in your dts like this:?
> 
>     &lcdif_out_mipi_dsi {
>         remote-endpoint = <&lcd_panel_in>;
>     };
> 
> I'm not sure what is the best default solution here for imx7s.dtsi. Also
> the labels don't work out in that case, this could be improved.

For sure for something to be defined a solution it should not introduce
regressions :-)

This commit makes other boards not work anymore, specifically this
reports is about colibri-imx7.

With that said, the following patch solves the issue

diff --git a/arch/arm/boot/dts/nxp/imx/imx7-colibri.dtsi b/arch/arm/boot/dts/nxp/imx/imx7-colibri.dtsi
index 9fe51884af79..966ad13e7c78 100644
--- a/arch/arm/boot/dts/nxp/imx/imx7-colibri.dtsi
+++ b/arch/arm/boot/dts/nxp/imx/imx7-colibri.dtsi
@@ -536,12 +536,15 @@ &lcdif {
        status = "disabled";

        port {
-               lcdif_out: endpoint {
+               lcdif_out: endpoint@0 {
+                       reg = <0>;
                        remote-endpoint = <&lcd_panel_in>;
                };
        };
 };

+/delete-node/ &mipi_dsi_in_lcdif;
+
 /* Colibri PWM<A> */
 &pwm1 {
        pinctrl-names = "default";


However, ... I do not really like the delete node, but it's required to
prevent this error:

arch/arm/boot/dts/nxp/imx/imx7s.dtsi:908.37-911.9: Warning (graph_endpoint): /soc/bus@30400000/dsi@30760000/ports/port@0/endpoint@0: graph connection to node '/soc/bus@30400000/lcdif@30730000/port/endpoint@0' is not bidirectional

With that said, unless you have a better solution I would just send a
revert for your change.

Francesco