Re: [PATCH v2 1/2] dt-bindings: display: Add Sharp Memory LCD bindings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 26/07/2024 21:44, Alex Lanzano wrote:
> Add device tree bindings for the monochrome Sharp Memory LCD
> 
> Signed-off-by: Alex Lanzano <lanzano.alex@xxxxxxxxx>
> Co-developed-by: Mehdi Djait <mehdi.djait@xxxxxxxxxxx>
> Signed-off-by: Mehdi Djait <mehdi.djait@xxxxxxxxxxx>

The order of tags is incorrect. Who developed it first? Please read at
Submitting patches - it explained this case quite precisely.

> ---
>  .../bindings/display/sharp,ls010b7dh04.yaml   | 94 +++++++++++++++++++
>  1 file changed, 94 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/sharp,ls010b7dh04.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/sharp,ls010b7dh04.yaml b/Documentation/devicetree/bindings/display/sharp,ls010b7dh04.yaml
> new file mode 100644
> index 000000000000..79bde7bf0d7d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/sharp,ls010b7dh04.yaml
> @@ -0,0 +1,94 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/sharp,ls010b7dh04.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Sharp Memory LCD panels
> +
> +maintainers:
> +  - Alex Lanzano <lanzano.alex@xxxxxxxxx>
> +
> +description:
> +  Sharp Memory LCDs are a series of monochrome displays that operate over
> +  a SPI bus when the chip select is high. The displays require a signal (VCOM)
> +  to be generated to prevent DC bias build up resulting in pixels being
> +  unable to change. Three modes can be used to provide the VCOM signal
> +  ("software", "external", "pwm").
> +
> +properties:
> +  compatible:
> +    enum:
> +      - sharp,ls010b7dh04
> +      - sharp,ls011b7dh03
> +      - sharp,ls012b7dd01
> +      - sharp,ls013b7dh03
> +      - sharp,ls013b7dh05
> +      - sharp,ls018b7dh02
> +      - sharp,ls027b7dh01
> +      - sharp,ls027b7dh01a
> +      - sharp,ls032b7dd02
> +      - sharp,ls044q7dh01
> +
> +  reg:
> +    maxItems: 1
> +
> +  spi-cs-high: true

<form letter>
This is a friendly reminder during the review process.

It seems my or other reviewer's previous comments were not fully
addressed. Maybe the feedback got lost between the quotes, maybe you
just forgot to apply it. Please go back to the previous discussion and
either implement all requested changes or keep discussing them.

Thank you.
</form letter>

> +
> +  spi-max-frequency:
> +    maximum: 2000000
> +
> +  sharp,vcom-mode:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    description: |
> +      software - This mode relies on a software operation to send a
> +      "maintain display" message to the display, toggling the vcom
> +      bit on and off with each message
> +
> +      external - This mode relies on an external clock to generate
> +      the signal on the EXTCOMM pin

External clock? Then you might be missing clocks property.
> +
> +      pwm - This mode relies on a pwm device to generate the signal
> +      on the EXTCOMM pin

That's an enum. Otherwise why "pony" would be a correct vcom-mode?

> +
> +  enable-gpios: true
> +
> +  pwms:
> +    maxItems: 1
> +    description: External VCOM signal
> +
> +required:
> +  - compatible
> +  - reg
> +  - spi-cs-high
> +  - sharp,vcom-mode
> +
> +allOf:
> +  - $ref: panel/panel-common.yaml#
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +  - if:
> +      properties:
> +        sharp,vcom-mode:
> +          const: pwm
> +    then:
> +      required:
> +        - pwms
> +
> +additionalProperties: false

Instead:
unevaluatedProperties: false

> +
> +examples:
> +  - |
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        display@0{

Missing space (see DTS coding style or any DTS)

> +            compatible = "sharp,ls013b7dh03";
> +            reg = <0>;
> +            spi-cs-high;
> +            spi-max-frequency = <1000000>;
> +            sharp,vcom-mode = "software";
> +        };
> +    };
> +...

Best regards,
Krzysztof




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux