[PATCH 1/2] ARM: dts: imx6sl: Fix lcdif compatible fallback

[PATCH 1/2] ARM: dts: imx6sl: Fix lcdif compatible fallback

[Fabio Estevam]

From: Fabio Estevam <festevam@denx.de>

According to fsl,lcdif.yaml, the correct imx6sl and imx6sll lcdif
compatible fallback is 'fsl,imx6sx-lcdif'.

Change it accordingly to fix the following dt-schema warnings:

['fsl,imx6sl-lcdif', 'fsl,imx28-lcdif'] is too long
['fsl,imx6sll-lcdif', 'fsl,imx28-lcdif'] is too long

Signed-off-by: Fabio Estevam <festevam@denx.de>
---
 arch/arm/boot/dts/nxp/imx/imx6sl.dtsi  | 2 +-
 arch/arm/boot/dts/nxp/imx/imx6sll.dtsi | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/nxp/imx/imx6sl.dtsi b/arch/arm/boot/dts/nxp/imx/imx6sl.dtsi
index 6aa61235e39e..c213f48382d5 100644
--- a/arch/arm/boot/dts/nxp/imx/imx6sl.dtsi
+++ b/arch/arm/boot/dts/nxp/imx/imx6sl.dtsi
@@ -773,7 +773,7 @@ epdc: epdc@20f4000 {
 			};
 
 			lcdif: lcdif@20f8000 {
-				compatible = "fsl,imx6sl-lcdif", "fsl,imx28-lcdif";
+				compatible = "fsl,imx6sl-lcdif", "fsl,imx6sx-lcdif";
 				reg = <0x020f8000 0x4000>;
 				interrupts = <0 39 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6SL_CLK_LCDIF_PIX>,
diff --git a/arch/arm/boot/dts/nxp/imx/imx6sll.dtsi b/arch/arm/boot/dts/nxp/imx/imx6sll.dtsi
index d219c2b67294..156cba79268b 100644
--- a/arch/arm/boot/dts/nxp/imx/imx6sll.dtsi
+++ b/arch/arm/boot/dts/nxp/imx/imx6sll.dtsi
@@ -660,7 +660,7 @@ pxp: pxp@20f0000 {
 			};
 
 			lcdif: lcd-controller@20f8000 {
-				compatible = "fsl,imx6sll-lcdif", "fsl,imx28-lcdif";
+				compatible = "fsl,imx6sll-lcdif", "fsl,imx6sx-lcdif";
 				reg = <0x020f8000 0x4000>;
 				interrupts = <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
 				clocks = <&clks IMX6SLL_CLK_LCDIF_PIX>,
-- 
2.34.1

[PATCH 2/2] ARM: dts: imx6sx: Remove fsl,imx6sx-lcdif fallback

[Fabio Estevam]

From: Fabio Estevam <festevam@denx.de>

According to fsl,lcdif.yaml, "fsl,imx6sx-lcdif" does not have any fallback.

Change it accordingly to fix the following dt-schema warnings:

['fsl,imx6sx-lcdif', 'fsl,imx28-lcdif'] is too long

Signed-off-by: Fabio Estevam <festevam@denx.de>
---
 arch/arm/boot/dts/nxp/imx/imx6sx.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/nxp/imx/imx6sx.dtsi b/arch/arm/boot/dts/nxp/imx/imx6sx.dtsi
index b386448486df..e56fcd9ee6f9 100644
--- a/arch/arm/boot/dts/nxp/imx/imx6sx.dtsi
+++ b/arch/arm/boot/dts/nxp/imx/imx6sx.dtsi
@@ -1301,7 +1301,7 @@ csi2: csi@221c000 {
 				};
 
 				lcdif1: lcdif@2220000 {
-					compatible = "fsl,imx6sx-lcdif", "fsl,imx28-lcdif";
+					compatible = "fsl,imx6sx-lcdif";
 					reg = <0x02220000 0x4000>;
 					interrupts = <GIC_SPI 5 IRQ_TYPE_EDGE_RISING>;
 					clocks = <&clks IMX6SX_CLK_LCDIF1_PIX>,
@@ -1322,7 +1322,7 @@ lcdif1_to_ldb: endpoint {
 				};
 
 				lcdif2: lcdif@2224000 {
-					compatible = "fsl,imx6sx-lcdif", "fsl,imx28-lcdif";
+					compatible = "fsl,imx6sx-lcdif";
 					reg = <0x02224000 0x4000>;
 					interrupts = <GIC_SPI 6 IRQ_TYPE_EDGE_RISING>;
 					clocks = <&clks IMX6SX_CLK_LCDIF2_PIX>,
-- 
2.34.1

Re: [PATCH 2/2] ARM: dts: imx6sx: Remove fsl,imx6sx-lcdif fallback

[Andreas Kemnade]

Hi,

Am Mon, 28 Oct 2024 15:08:44 -0300
schrieb Fabio Estevam <festevam@gmail.com>:

> From: Fabio Estevam <festevam@denx.de>
> 
> According to fsl,lcdif.yaml, "fsl,imx6sx-lcdif" does not have any
> fallback.
> 
> Change it accordingly to fix the following dt-schema warnings:
> 
> ['fsl,imx6sx-lcdif', 'fsl,imx28-lcdif'] is too long
> 
> Signed-off-by: Fabio Estevam <festevam@denx.de>
> ---

As the devicetree might also be used with other software (e.g. u-boot),
this might break something.  So if u-boot (or any other software) does
work with fsl,imx28-lcdif because it only uses a subset of features of
fsl,imx6sx, it might be worth changing the binding instead.

Same for Patch 1. But I cannot test that and do not have a strong
opinion here.

But thanks for touching this annoying warning.

Regards,
Andreas

Re: [PATCH 2/2] ARM: dts: imx6sx: Remove fsl,imx6sx-lcdif fallback

[Fabio Estevam]

Hi Andreas,

On Mon, Oct 28, 2024 at 3:47 PM Andreas Kemnade <andreas@kemnade.info> wrote:

> As the devicetree might also be used with other software (e.g. u-boot),
> this might break something.  So if u-boot (or any other software) does
> work with fsl,imx28-lcdif because it only uses a subset of features of
> fsl,imx6sx, it might be worth changing the binding instead.
>
> Same for Patch 1. But I cannot test that and do not have a strong
> opinion here.

U-Boot would not be broken after these series:

https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/video/mxsfb.c?ref_type=heads#L388

Re: [PATCH 2/2] ARM: dts: imx6sx: Remove fsl,imx6sx-lcdif fallback

[Ahmad Fatoum]

Hi,

[Cc += Marek, who maintains the Linux driver]

On 28.10.24 20:48, Fabio Estevam wrote:
> Hi Andreas,
> 
> On Mon, Oct 28, 2024 at 3:47 PM Andreas Kemnade <andreas@kemnade.info> wrote:
> 
>> As the devicetree might also be used with other software (e.g. u-boot),
>> this might break something.  So if u-boot (or any other software) does
>> work with fsl,imx28-lcdif because it only uses a subset of features of
>> fsl,imx6sx, it might be worth changing the binding instead.

Thanks for raising this point, Andreas. I think it's important to adjust
the binding's compatible list binding if need be to avoid breaking DT
consumers.

>> Same for Patch 1. But I cannot test that and do not have a strong
>> opinion here.
> 
> U-Boot would not be broken after these series:
> 
> https://source.denx.de/u-boot/u-boot/-/blob/master/drivers/video/mxsfb.c?ref_type=heads#L388

The barebox driver only matches against fsl,imx23-lcdif and fsl,imx28-lcdif:
https://elixir.bootlin.com/barebox/v2024.10.0/source/drivers/video/stm.c#L579

The MXSFB IP appears to be completely backwards compatible. Otherwise the
i.MX6SL/i.MX6SLL integration of it wouldn't have worked as it used to match
against imx28-lcdif so far. Checking the Linux driver, the differences to
the i.MX6SX also look like they are not backwards incompatible.

On the other hand, Linux users may start to make use of the new features
that aren't available without having imx6sx-lcdif in the compatible list,
like the overlay plane and the CRC32 functionality.

With an eye towards improving device tree stability, I think it's more
appropriate to adjust the binding to have three compatibles instead.

Thanks,
Ahmad

> 
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 2/2] ARM: dts: imx6sx: Remove fsl,imx6sx-lcdif fallback

[Fabio Estevam]

Hi Ahmad,

On Tue, Oct 29, 2024 at 6:50 AM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:

> With an eye towards improving device tree stability, I think it's more
> appropriate to adjust the binding to have three compatibles instead.

Yes, this makes sense. I will adjust the bindings accordingly.

Thanks