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

[Rob Herring <robh@kernel.org>]

On Mon, Aug 17, 2020 at 03:04:06AM +0300, Laurent Pinchart wrote:
> 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.

Because I haven't integrated yamllint yet. I do have a config file at 
least.

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