Re: [PATCH 1/3] drm/mxsfb: Add an entry for "fsl,imx8mq-lcdif"

[Lucas Stach <l.stach@pengutronix.de>]

Hi Fabio,

Am Montag, dem 11.12.2023 um 17:41 -0300 schrieb Fabio Estevam:
> From: Fabio Estevam <festevam@denx.de>
> 
> On i.MX6SX, the LCDIF has an associated power domain.
> 
> However, i.MX8MQ does not have an LCDIF power domain.
> 
> imx8mq.dtsi has the following compatible string:
> 
> compatible = "fsl,imx8mq-lcdif", "fsl,imx6sx-lcdif";
> 
> which causes the following dt-schema warning:
> 
> imx8mq-evk.dtb: lcd-controller@30320000: 'power-domains' is a required property
> 	from schema $id: http://devicetree.org/schemas/display/fsl,lcdif.yaml#
> 
> To prevent this problem, add a specific fsl,imx8mq-lcdif entry in
> the driver to properly handle such a power-domain requirement difference.
> 
I don't really like this series. While we don't make any strong
guarantees in this way, it breaks booting older kernels with a new DT.

As this driver patch shows, there are no actual differences in the IP
block between i.MX6SX and i.MX8MQ, it's just the SoC integration that's
different, where in one SoC the controller is located in a power
domain, while it isn't in another.

To me this smells like making changes to parts that don't really care
about those differences. The thing that cares about the difference is
the DT schema validation and I feel like there must be a better way to
describe this in the schema than to patch both driver and DT just to
accommodate a SoC integration difference. Is there a way to make the
power-domain property not dependent on the IP block, but the machine
compatible?

Regards,
Lucas

> Signed-off-by: Fabio Estevam <festevam@denx.de>
> ---
>  drivers/gpu/drm/mxsfb/mxsfb_drv.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> index b483ef48216a..ac9ce3b45b38 100644
> --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c
> @@ -340,6 +340,7 @@ static const struct of_device_id mxsfb_dt_ids[] = {
>  	{ .compatible = "fsl,imx23-lcdif", .data = &mxsfb_devdata[MXSFB_V3], },
>  	{ .compatible = "fsl,imx28-lcdif", .data = &mxsfb_devdata[MXSFB_V4], },
>  	{ .compatible = "fsl,imx6sx-lcdif", .data = &mxsfb_devdata[MXSFB_V6], },
> +	{ .compatible = "fsl,imx8mq-lcdif", .data = &mxsfb_devdata[MXSFB_V6], },
>  	{ /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, mxsfb_dt_ids);