On Sat, Jul 27, 2024 at 11:06:05AM GMT, Krzysztof Kozlowski wrote: > 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. > Will fix! > > --- > > .../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> > My apologies! I must've missed the comment on this line. I replied to this on the v1 thread > > + > > + 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. This is to handle the case where a clock IC or other signal generator IC is provided on the display module itself. So I don't believe the clocks property would be needed. > > + > > + 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? > Will fix! > > + > > + 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 > Will fix! > > + > > +examples: > > + - | > > + spi { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + display@0{ > > Missing space (see DTS coding style or any DTS) > Will Fix! > > + compatible = "sharp,ls013b7dh03"; > > + reg = <0>; > > + spi-cs-high; > > + spi-max-frequency = <1000000>; > > + sharp,vcom-mode = "software"; > > + }; > > + }; > > +... > > Best regards, > Krzysztof > Again, thank you for taking the time to review. Best regards, Alex