Re: [PATCH v2 4/4] dt-bindings: display/panel: Add Leadtek ltk035c5444t

[Paul Cercueil]

Hi Rob,

Le jeu., mars 10 2022 at 16:30:26 -0600, Rob Herring <robh@xxxxxxxxxx> 
a écrit :
> On Tue, Mar 08, 2022 at 02:06:43PM +0100, Christophe Branchereau 
> wrote:
> >  Add binding for the leadtek ltk035c5444t, which is a 640x480
> >  mipi-dbi over spi / 24-bit RGB panel based on the newvision
> >  NV03052C chipset.
> >
> >  It is found in the Anbernic RG350M mips handheld.
> >
> >  Signed-off-by: Christophe Branchereau <cbranchereau@xxxxxxxxx>
> >  ---
> >   .../panel/leadtek,ltk035c5444t-spi.yaml       | 59 
> > +++++++++++++++++++
> >   1 file changed, 59 insertions(+)
> >   create mode 100644 
> > Documentation/devicetree/bindings/display/panel/leadtek,ltk035c5444t-spi.yaml
>
> We have 18 SPI based panels already:
>
> $ git grep -i 'spi.* {' 
> Documentation/devicetree/bindings/display/panel/
> Documentation/devicetree/bindings/display/panel/abt,y030xx067a.yaml:  
>   spi {
> Documentation/devicetree/bindings/display/panel/ilitek,ili9163.yaml:  
>   spi {
> Documentation/devicetree/bindings/display/panel/ilitek,ili9322.yaml:  
>   spi {
> Documentation/devicetree/bindings/display/panel/ilitek,ili9341.yaml:  
>   spi {
> Documentation/devicetree/bindings/display/panel/innolux,ej030na.yaml: 
>    spi {
> Documentation/devicetree/bindings/display/panel/kingdisplay,kd035g6-54nt.yaml: 
>    spi {
> Documentation/devicetree/bindings/display/panel/lg,lg4573.yaml:    
> spi {
> Documentation/devicetree/bindings/display/panel/lgphilips,lb035q02.yaml: 
>    spi {
> Documentation/devicetree/bindings/display/panel/nec,nl8048hl11.yaml:  
>   spi0 {
> Documentation/devicetree/bindings/display/panel/samsung,ld9040.yaml:  
>   spi {
> Documentation/devicetree/bindings/display/panel/samsung,lms380kf01.yaml: 
>    spi {
> Documentation/devicetree/bindings/display/panel/samsung,lms397kf04.yaml: 
>    spi {
> Documentation/devicetree/bindings/display/panel/samsung,s6d27a1.yaml: 
>    spi {
> Documentation/devicetree/bindings/display/panel/samsung,s6e63m0.yaml: 
>    spi {
> Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml: 
>    spi {
> Documentation/devicetree/bindings/display/panel/sony,acx565akm.yaml:  
>   spi {
> Documentation/devicetree/bindings/display/panel/tpo,td.yaml:    spi {
> Documentation/devicetree/bindings/display/panel/tpo,tpg110.yaml:    
> spi {
>
> Most except for the Samsung ones look like they'd fit in our 
> definition
> of 'simple panels' which primarily means 1 supply.
>
> So I think it is time for a panel-simple-spi.yaml binding to combine 
> all
> these. But I'm not going to make the person adding the 19th case to do
> that, and this otherwise looks fine:
>
> Reviewed-by: Rob Herring <robh@xxxxxxxxxx>

I actually thought you sent a patch for this already (I remember you 
talking about it last a binding for a new SPI panel has been added).

I will work on it next week - provided Cristophe's patch has been 
merged before.

Cheers,
-Paul

> With one nit fixed below:
>
> >  diff --git 
> > a/Documentation/devicetree/bindings/display/panel/leadtek,ltk035c5444t-spi.yaml 
> > b/Documentation/devicetree/bindings/display/panel/leadtek,ltk035c5444t-spi.yaml
> >  new file mode 100644
> >  index 000000000000..9b6f1810adab
> >  --- /dev/null
> >  +++ 
> > b/Documentation/devicetree/bindings/display/panel/leadtek,ltk035c5444t-spi.yaml
> >  @@ -0,0 +1,59 @@
> >  +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >  +%YAML 1.2
> >  +---
> >  +$id: 
> > http://devicetree.org/schemas/display/panel/leadtek,ltk035c5444t-spi.yaml#
> >  +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >  +
> >  +title: Leadtek ltk035c5444t 3.5" (640x480 pixels) 24-bit IPS LCD 
> > panel
> >  +
> >  +maintainers:
> >  +  - Paul Cercueil <paul@xxxxxxxxxxxxxxx>
> >  +  - Christophe Branchereau <cbranchereau@xxxxxxxxx>
> >  +
> >  +allOf:
> >  +  - $ref: panel-common.yaml#
> >  +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> >  +
> >  +properties:
> >  +  compatible:
> >  +    const: leadtek,ltk035c5444t-spi
>
> '-spi' is redundant, so drop.
>
> >  +
> >  +  backlight: true
> >  +  port: true
> >  +  power-supply: true
> >  +  reg: true
> >  +  reset-gpios: true
> >  +
> >  +required:
> >  +  - compatible
> >  +  - power-supply
> >  +  - reset-gpios
> >  +
> >  +unevaluatedProperties: false
> >  +
> >  +examples:
> >  +  - |
> >  +    #include <dt-bindings/gpio/gpio.h>
> >  +
> >  +    spi {
> >  +        #address-cells = <1>;
> >  +        #size-cells = <0>;
> >  +        panel@0 {
> >  +            compatible = "leadtek,ltk035c5444t-spi";
>
> And update the example...
>
> >  +            reg = <0>;
> >  +
> >  +            spi-3wire;
> >  +            spi-max-frequency = <3125000>;
> >  +
> >  +            reset-gpios = <&gpe 2 GPIO_ACTIVE_LOW>;
> >  +
> >  +            backlight = <&backlight>;
> >  +            power-supply = <&vcc>;
> >  +
> >  +            port {
> >  +                panel_input: endpoint {
> >  +                    remote-endpoint = <&panel_output>;
> >  +                };
> >  +            };
> >  +        };
> >  +    };
> >  --
> >  2.34.1