| Message ID | 20231211204138.553141-1-festevam@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] drm/mxsfb: Add an entry for "fsl,imx8mq-lcdif" | expand |
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);
Hi Lucas, On Tue, Dec 12, 2023 at 3:19 PM Lucas Stach <l.stach@pengutronix.de> wrote: > 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. I thought we needed only to guarantee that old DTs still run with newer kernels, not the other way around. > 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? Yes, this series is all about making dt-schema validation happy. If there is a better alternative, I am all ears. Rob, Krzysztof, Conor Any suggestions? Thanks
Hi Fabio, Am Dienstag, dem 12.12.2023 um 15:28 -0300 schrieb Fabio Estevam: > Hi Lucas, > > On Tue, Dec 12, 2023 at 3:19 PM Lucas Stach <l.stach@pengutronix.de> wrote: > > > 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. > > I thought we needed only to guarantee that old DTs still run with > newer kernels, not the other way around. > That's right. At least in the i.MX world we never made a strong guarantee in that way. Still we should not break it deliberately if there isn't a good reason to do so. And I _feel_ that "DT schema isn't able to express the SoC integration" isn't a good enough reason to break things. But maybe there is a way to describe this in the DT schema, that we aren't aware of yet. I'm not quite up-to-date with all the DT schema stuff... Regards, Lucas
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);