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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <52bf1aa6-449f-4f77-992e-45e510fe6e7f@kernel.org>
Date: Sat, 27 Jul 2024 11:06:05 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Alex Lanzano <lanzano.alex@...il.com>, mehdi.djait@...tlin.com,
 christophe.jaillet@...adoo.fr,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
 David Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>
Cc: dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/2] dt-bindings: display: Add Sharp Memory LCD
 bindings

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@...il.com>
> Co-developed-by: Mehdi Djait <mehdi.djait@...tlin.com>
> Signed-off-by: Mehdi Djait <mehdi.djait@...tlin.com>

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@...il.com>
> +
> +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