Re: [DO NOT MERGE v5 22/37] dt-bindings: display: smi,sm501: SMI SM501 binding json-schema

[Arnd Bergmann]

On Tue, Dec 5, 2023, at 10:45, Yoshinori Sato wrote:
> Define SM501 functions and modes.
>
> Signed-off-by: Yoshinori Sato <ysato@xxxxxxxxxxxxxxxxxxxx>
> ---
>  .../bindings/display/smi,sm501.yaml           | 134 ++++++++++++++++++
>  include/dt-bindings/display/sm501.h           |  25 ++++

It looks like we already have a binding at
Documentation/devicetree/bindings/display/sm501fb.txt

> +  little-endian:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description: available on big endian systems, to set different 
> foreign endian.
> +  big-endian:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description: available on little endian systems, to set different 
> foreign endian.
> +
> +  swap-fb-endian:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description: swap framebuffer byteorder.

Why do you need both the "swap" and the specific little/big
properties?

> +  crt:
> +    description: CRT output control
> +
> +  panel:
> +    description: Panel output control

What type are these?

> +  smi,misc-timing:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Miscellaneous Timing reg value.
> +
> +  smi,misc-control:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Miscellaneous Control reg value.
> +
> +  smi,gpio-low:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: GPIO0 to 31 Control reg value.
> +
> +  smi,gpio-high:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: GPIO32 to 63 Control reg value.

Register values should generally not go into DT


> +
> +  smi,gpio-i2c:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 5
> +    description: |
> +      GPIO I2C bus number
> +      1st field - I2C bus number
> +      2nd Field - GPIO SDA
> +      3rd Field - GPIO SCL
> +      4th Field - Timeout
> +      5th Field - udelay

Instead of a bus number and other fields, I think
this should reference an i2c device.

      Arnd