Re: [PATCH 2/2] arm64: dts: imx8mp-verdin: add dsi to lvds functionality

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Max Krummenacher <max.oss.09@gmail.com>
Cc: Max Krummenacher <max.krummenacher@toradex.com>,
	Fabio Estevam <festevam@gmail.com>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Marcel Ziswiler <marcel.ziswiler@toradex.com>,
	NXP Linux Team <linux-imx@nxp.com>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Philippe Schenker <philippe.schenker@toradex.com>,
	Rob Herring <robh+dt@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Shawn Guo <shawnguo@kernel.org>,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] arm64: dts: imx8mp-verdin: add dsi to lvds functionality
Date: Thu, 1 Sep 2022 21:10:25 +0300	[thread overview]
Message-ID: <YxD1kecBOaqG+GNh@pendragon.ideasonboard.com> (raw)
In-Reply-To: <20220901154051.1885509-2-max.oss.09@gmail.com>

Hi Max,

Thank you for the patch.

On Thu, Sep 01, 2022 at 05:40:51PM +0200, Max Krummenacher wrote:
> From: Max Krummenacher <max.krummenacher@toradex.com>
> 
> Add a panel-lvds node and use the correct dsi to lvds chip name.
> Both to be later extended in a dt overlay according to the exact
> board HW configuration.

Same as with patch 1/2, this doesn't look right. The panel isn't part of
the Verdin SoM, it's not even part of the base board. It should be moved
to an overlay, and the SN65DSI83, which I understand isn't on the SoM
either, should be moved somewhere else too..

> Signed-off-by: Max Krummenacher <max.krummenacher@toradex.com>
> ---
> 
>  arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi
> index bd84a0d135dc..a3e20c7add3e 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-verdin.dtsi
> @@ -67,6 +67,13 @@ hdmi_connector: hdmi-connector {
>  		status = "disabled";
>  	};
>  
> +	panel_lvds: panel-lvds {
> +		compatible = "panel-lvds";
> +		backlight = <&backlight>;
> +		data-mapping = "vesa-24";
> +		status = "disabled";
> +	};
> +
>  	/* Carrier Board Supplies */
>  	reg_1p8v: regulator-1p8v {
>  		compatible = "regulator-fixed";
> @@ -690,8 +697,8 @@ gpio_expander_21: gpio-expander@21 {
>  		status = "disabled";
>  	};
>  
> -	lvds_ti_sn65dsi83: bridge@2c {
> -		compatible = "ti,sn65dsi83";
> +	lvds_ti_sn65dsi84: bridge@2c {
> +		compatible = "ti,sn65dsi84";
>  		/* Verdin GPIO_9_DSI (SN65DSI84 IRQ, SODIMM 17, unused) */
>  		/* Verdin GPIO_10_DSI (SODIMM 21) */
>  		enable-gpios = <&gpio4 28 GPIO_ACTIVE_HIGH>;

-- 
Regards,

Laurent Pinchart

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel