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

Commit Message

Fabio Estevam Oct. 28, 2024, 6:08 p.m. UTC

From: Fabio Estevam <[e-mail]>

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 <[e-mail]>
---
 arch/arm/boot/dts/nxp/imx/imx6sx.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Andreas Kemnade Oct. 28, 2024, 6:47 p.m. UTC | #1

Hi,

Am Mon, 28 Oct 2024 15:08:44 -0300
schrieb Fabio Estevam <[e-mail]>:

> From: Fabio Estevam <[e-mail]>
> 
> 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 <[e-mail]>
> ---

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

Fabio Estevam Oct. 28, 2024, 7:48 p.m. UTC | #2

Hi Andreas,

On Mon, Oct 28, 2024 at 3:47 PM Andreas Kemnade <[e-mail]> 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

Ahmad Fatoum Oct. 29, 2024, 9:50 a.m. UTC | #3

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 <[e-mail]> 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

> 
>

Fabio Estevam Oct. 29, 2024, 11:17 a.m. UTC | #4

Hi Ahmad,

On Tue, Oct 29, 2024 at 6:50 AM Ahmad Fatoum <[e-mail]> 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

Patch

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>,