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

[Paul Cercueil <paul@crapouillou.net>]

Hi Rob,

Le jeu., mars 10 2022 at 16:30:26 -0600, Rob Herring <robh@kernel.org> 
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@gmail.com>
>>  ---
>>   .../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@kernel.org>

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@crapouillou.net>
>>  +  - Christophe Branchereau <cbranchereau@gmail.com>
>>  +
>>  +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
>> 
>>