Re: [PATCH 2/8] dt-bindings: display: mxsfb: Add and fix compatible strings

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Sam,

On Sun, Aug 16, 2020 at 08:39:33AM +0200, Sam Ravnborg wrote:
> On Thu, Aug 13, 2020 at 04:29:04AM +0300, Laurent Pinchart wrote:
> > Additional compatible strings have been added in DT source for the
> > i.MX6SL, i.MX6SLL, i.MX6UL and i.MX7D without updating the bindings.
> > Most of the upstream DT sources use the fsl,imx28-lcdif compatible
> > string, which mostly predates the realization that the LCDIF in the
> > i.MX6 and newer SoCs have extra features compared to the i.MX28.
> > 
> > Update the bindings to add the missing compatible strings, with the
> > correct fallback values. This fails to validate some of the upstream DT
> > sources. Instead of adding the incorrect compatible fallback to the
> > binding, the sources should be updated separately.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  .../devicetree/bindings/display/mxsfb.yaml     | 18 +++++++++++++-----
> >  1 file changed, 13 insertions(+), 5 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/display/mxsfb.yaml b/Documentation/devicetree/bindings/display/mxsfb.yaml
> > index 202381ec5bb7..ec6533b1d4a3 100644
> > --- a/Documentation/devicetree/bindings/display/mxsfb.yaml
> > +++ b/Documentation/devicetree/bindings/display/mxsfb.yaml
> > @@ -15,11 +15,19 @@ description: |
> >  
> >  properties:
> >    compatible:
> > -    enum:
> > -      - fsl,imx23-lcdif
> > -      - fsl,imx28-lcdif
> > -      - fsl,imx6sx-lcdif
> > -      - fsl,imx8mq-lcdif
> > +    oneOf:
> > +      - enum:
> > +          - fsl,imx23-lcdif
> > +          - fsl,imx28-lcdif
> > +          - fsl,imx6sx-lcdif
>
> Indent correct.
> 
> > +      - items:
> > +        - enum:
> > +          - fsl,imx6sl-lcdif
> > +          - fsl,imx6sll-lcdif
> > +          - fsl,imx6ul-lcdif
> > +          - fsl,imx7d-lcdif
> > +          - fsl,imx8mq-lcdif
>
> Indent shall be two more spaces like above.
> (This is at least my best uderstanding)

I think you're right. I wonder why dt_binding_check doesn't complain.
I'll fix it anyway.

> > +        - const: fsl,imx6sx-lcdif
> 
> With the above the following compatibles are supported:
> 
> 	"fsl,imx23-lcdif"
> 	"fsl,imx28-lcdif"
> 	"fsl,imx6sx-lcdif"
> 	"fsl,imx8mq-lcdif"
> 
> 	"fsl,imx6sl-lcdif", "fsl,imx6sx-lcdif"
> 	"fsl,imx6sll-lcdif", "fsl,imx6sx-lcdif"
> 	"fsl,imx6ul-lcdif", "fsl,imx6sx-lcdif"
> 	"fsl,imx7d-lcdif", "fsl,imx6sx-lcdif"
> 	"fsl,imx8mq-lcdif", "fsl,imx6sx-lcdif"
> 
> So the fallback value is the later "fsl,imx6sx-lcdif" which looks ok.
> 
> With indent fixed (or explained why I am wrong):
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
> 
> >  
> >    reg:
> >      maxItems: 1

-- 
Regards,

Laurent Pinchart