| Message ID | 20241029191640.379315-2-festevam@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3,1/3] dt-bindings: lcdif: Document a imx6sx-lcdif fallback | expand |
On 10/29/24 8:16 PM, Fabio Estevam wrote: > From: Fabio Estevam <festevam@denx.de> > > mx6sl.dtsi and imx6sll.dtsi have the following lcdif entries: > > compatible = "fsl,imx6sl-lcdif", "fsl,imx28-lcdif"; > > This causes dt-schema warnings as the current binding only > allow 'fsl,imx6sx-lcdif' as fallback. > > ['fsl,imx6sl-lcdif', 'fsl,imx28-lcdif'] is too long > ['fsl,imx6sll-lcdif', 'fsl,imx28-lcdif'] is too long > > The imx6sx-lcdif programming model has more advanced features, such > as overlay plane and the CRC32 support than the imx28-lcdif IP. > > Expand the imx6sl/imx6sll lcdif fallbacks to accept a less specific > fsl,imx28-lcdif fallback: > > compatible = "fsl,imx6sl-lcdif", "fsl,imx6sx-lcdif", "fsl,imx28-lcdif"; > > This helps keeping DT compatibility as well as using the more advanced > lcdif features found on imx6sl and imx6sll. > > Signed-off-by: Fabio Estevam <festevam@denx.de> > --- > Changes since v2: > - Make the three compatible entres the only valid combination > for imx6sl and imx6sll (Andreas). > > Documentation/devicetree/bindings/display/fsl,lcdif.yaml | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml > index ad0cca562463..72e509bc975b 100644 > --- a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml > +++ b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml > @@ -23,14 +23,18 @@ properties: > - fsl,imx93-lcdif > - items: > - enum: > - - fsl,imx6sl-lcdif > - - fsl,imx6sll-lcdif > - fsl,imx6ul-lcdif > - fsl,imx7d-lcdif > - fsl,imx8mm-lcdif > - fsl,imx8mn-lcdif > - fsl,imx8mq-lcdif > - const: fsl,imx6sx-lcdif > + - items: > + - enum: > + - fsl,imx6sl-lcdif > + - fsl,imx6sll-lcdif > + - const: fsl,imx6sx-lcdif > + - const: fsl,imx28-lcdif Shouldn't this be - enum: - fsl,imx6sl-lcdif - fsl,imx6sll-lcdif - fsl,imx6sx-lcdif - const: fsl,imx28-lcdif So you wouldn't have to write three compatible strings for the 6sl/sll , but only two ? I.e. this: compatible = "fsl,imx6sl-lcdif", "fsl,imx28-lcdif"; compatible = "fsl,imx6sll-lcdif", "fsl,imx28-lcdif"; compatible = "fsl,imx6sx-lcdif", "fsl,imx28-lcdif"; ?
On 29.10.24 21:16, Marek Vasut wrote: > On 10/29/24 8:16 PM, Fabio Estevam wrote: >> From: Fabio Estevam <festevam@denx.de> >> >> mx6sl.dtsi and imx6sll.dtsi have the following lcdif entries: >> >> compatible = "fsl,imx6sl-lcdif", "fsl,imx28-lcdif"; >> >> This causes dt-schema warnings as the current binding only >> allow 'fsl,imx6sx-lcdif' as fallback. >> >> ['fsl,imx6sl-lcdif', 'fsl,imx28-lcdif'] is too long >> ['fsl,imx6sll-lcdif', 'fsl,imx28-lcdif'] is too long >> >> The imx6sx-lcdif programming model has more advanced features, such >> as overlay plane and the CRC32 support than the imx28-lcdif IP. >> >> Expand the imx6sl/imx6sll lcdif fallbacks to accept a less specific >> fsl,imx28-lcdif fallback: >> >> compatible = "fsl,imx6sl-lcdif", "fsl,imx6sx-lcdif", "fsl,imx28-lcdif"; >> >> This helps keeping DT compatibility as well as using the more advanced >> lcdif features found on imx6sl and imx6sll. >> >> Signed-off-by: Fabio Estevam <festevam@denx.de> >> --- >> Changes since v2: >> - Make the three compatible entres the only valid combination >> for imx6sl and imx6sll (Andreas). >> >> Documentation/devicetree/bindings/display/fsl,lcdif.yaml | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml >> index ad0cca562463..72e509bc975b 100644 >> --- a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml >> +++ b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml >> @@ -23,14 +23,18 @@ properties: >> - fsl,imx93-lcdif >> - items: >> - enum: >> - - fsl,imx6sl-lcdif >> - - fsl,imx6sll-lcdif >> - fsl,imx6ul-lcdif >> - fsl,imx7d-lcdif >> - fsl,imx8mm-lcdif >> - fsl,imx8mn-lcdif >> - fsl,imx8mq-lcdif >> - const: fsl,imx6sx-lcdif >> + - items: >> + - enum: >> + - fsl,imx6sl-lcdif >> + - fsl,imx6sll-lcdif >> + - const: fsl,imx6sx-lcdif >> + - const: fsl,imx28-lcdif > Shouldn't this be > > - enum: > - fsl,imx6sl-lcdif > - fsl,imx6sll-lcdif > - fsl,imx6sx-lcdif > - const: fsl,imx28-lcdif > > So you wouldn't have to write three compatible strings for the 6sl/sll , but only two ? I.e. this: > > compatible = "fsl,imx6sl-lcdif", "fsl,imx28-lcdif"; > compatible = "fsl,imx6sll-lcdif", "fsl,imx28-lcdif"; > compatible = "fsl,imx6sx-lcdif", "fsl,imx28-lcdif"; This would necessitate changing the Linux driver to match against the fsl,imx6sl-lcdif & fsl,imx6sll-lcdif compatible as well, otherwise new features like e.g. the overlay plane won't be available. Cheers, Ahmad > > ? >
Hi Marek, On Tue, Oct 29, 2024 at 5:16 PM Marek Vasut <marex@denx.de> wrote: > So you wouldn't have to write three compatible strings for the 6sl/sll , > but only two ? I.e. this: > > compatible = "fsl,imx6sl-lcdif", "fsl,imx28-lcdif"; > compatible = "fsl,imx6sll-lcdif", "fsl,imx28-lcdif"; i.MX6SL and i.MX6SLL have a more advanced LCDIF version than the one on i.MX28. The LCDIF block on i.MX6SL/i.MX6SLL has both overlay and CRC support, corresponding to the MXSFB_V6 of the mxsfb_drv driver. That's why it is better to have the compatibles as: compatible = "fsl,imx6sl-lcdif", "fsl,imx6sx-lcdif", "fsl,imx28-lcdif"; compatible = "fsl,imx6sll-lcdif", "fsl,imx6sx-lcdif", "fsl,imx28-lcdif"; This way they can match with a more specific fsl,imx6sx-lcdif entry. If an OS does not implement the fsl,imx6sx-lcdif match, it can still match with fsl,imx28-lcdif as a fallback, keeping the DT compatibility. I missed updating the binding example. I will wait for more feedback before sending a v4. Thanks
diff --git a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml index ad0cca562463..72e509bc975b 100644 --- a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml +++ b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml @@ -23,14 +23,18 @@ properties: - fsl,imx93-lcdif - items: - enum: - - fsl,imx6sl-lcdif - - fsl,imx6sll-lcdif - fsl,imx6ul-lcdif - fsl,imx7d-lcdif - fsl,imx8mm-lcdif - fsl,imx8mn-lcdif - fsl,imx8mq-lcdif - const: fsl,imx6sx-lcdif + - items: + - enum: + - fsl,imx6sl-lcdif + - fsl,imx6sll-lcdif + - const: fsl,imx6sx-lcdif + - const: fsl,imx28-lcdif - items: - enum: - fsl,imx6sx-lcdif