| Message ID | 20250402-st7571-v1-1-351d6b9eeb4a@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for Sitronix ST7571 LCD controller | expand |
On Wed, Apr 02, 2025 at 08:12:10AM +0200, Marcus Folkesson wrote: > Sitronix ST7571 is a 4bit gray scale dot matrix LCD controller. > The controller has a SPI, I2C and 8bit parallel interface, this is for > the I2C interface only. > > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com> > --- > .../bindings/display/sitronix,st7571-i2c.yaml | 71 ++++++++++++++++++++++ > 1 file changed, 71 insertions(+) > > diff --git a/Documentation/devicetree/bindings/display/sitronix,st7571-i2c.yaml b/Documentation/devicetree/bindings/display/sitronix,st7571-i2c.yaml > new file mode 100644 > index 0000000000000000000000000000000000000000..6e5e0994a98db646a37bb17c4289332546c9266e > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/sitronix,st7571-i2c.yaml Drop i2c > @@ -0,0 +1,71 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/sitronix,st7571-i2c.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Sitronix ST7571 Display Panels > + > +maintainers: > + - Marcus Folkesson <marcus.folkesson@gmail.com> > + > +properties: > + compatible: > + const: sitronix,st7571-i2c Drop i2c > + > + reg: > + description: I2C address of the LCD controller Drop description > + maxItems: 1 > + > + sitronix,panel-width-mm: > + description: physical panel width [mm] > + > + sitronix,panel-height-mm: > + description: physical panel height [mm] No, use standard properties. > + > + sitronix,panel-nlines: > + description: Number of lines in the panel > + $ref: /schemas/types.yaml#/definitions/uint32 > + minimum: 1 > + maximum: 128 > + default: 128 Ditto > + > + sitronix,panel-start-line: > + description: Start line of the panel > + $ref: /schemas/types.yaml#/definitions/uint32 > + minimum: 0 > + maximum: 127 > + default: 0 Ditto > + > + reset-gpios: > + description: GPIO connected to the RST_N signal. > + maxItems: 1 > + > +required: > + - compatible > + - reg > + - reset-gpios Keep same order as in properties. > + - sitronix,panel-width-mm > + - sitronix,panel-height-mm > + Missing ref to panels schema. > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/gpio/gpio.h> > + # Drop > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + display@3f { Look how this is called in other bindings... The binding and example are not following existing code. Why? Why doing something entirely different? Best regards, Krzysztof
On Wed, Apr 02, 2025 at 08:12:10AM +0200, Marcus Folkesson wrote: > + display@3f { > + compatible = "sitronix,st7571-i2c"; > + reg = <0x3f>; > + > + reset-gpios = <&gpio0 3 GPIO_ACTIVE_HIGH>; so RST or RST_N? Best regards, Krzysztof
On 02/04/2025 08:12, Marcus Folkesson wrote: > Sitronix ST7571 is a 4bit gray scale dot matrix LCD controller. > The controller has a SPI, I2C and 8bit parallel interface, this is for > the I2C interface only. > > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com> I spotted one more thing: Please use subject prefixes matching the subsystem. You can get them for example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory your patch is touching. For bindings, the preferred subjects are explained here: https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters (missing display parts) A nit, subject: drop second/last, redundant "bindings". The "dt-bindings" prefix is already stating that these are bindings. See also: https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18 Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/display/sitronix,st7571-i2c.yaml b/Documentation/devicetree/bindings/display/sitronix,st7571-i2c.yaml new file mode 100644 index 0000000000000000000000000000000000000000..6e5e0994a98db646a37bb17c4289332546c9266e --- /dev/null +++ b/Documentation/devicetree/bindings/display/sitronix,st7571-i2c.yaml @@ -0,0 +1,71 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/sitronix,st7571-i2c.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Sitronix ST7571 Display Panels + +maintainers: + - Marcus Folkesson <marcus.folkesson@gmail.com> + +properties: + compatible: + const: sitronix,st7571-i2c + + reg: + description: I2C address of the LCD controller + maxItems: 1 + + sitronix,panel-width-mm: + description: physical panel width [mm] + + sitronix,panel-height-mm: + description: physical panel height [mm] + + sitronix,panel-nlines: + description: Number of lines in the panel + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 1 + maximum: 128 + default: 128 + + sitronix,panel-start-line: + description: Start line of the panel + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 0 + maximum: 127 + default: 0 + + reset-gpios: + description: GPIO connected to the RST_N signal. + maxItems: 1 + +required: + - compatible + - reg + - reset-gpios + - sitronix,panel-width-mm + - sitronix,panel-height-mm + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + # + i2c { + #address-cells = <1>; + #size-cells = <0>; + + display@3f { + compatible = "sitronix,st7571-i2c"; + reg = <0x3f>; + + reset-gpios = <&gpio0 3 GPIO_ACTIVE_HIGH>; + sitronix,panel-width-mm = <37>; + sitronix,panel-height-mm = <27>; + sitronix,panel-nlines = <96>; + sitronix,panel-start-line = <0>; + }; + };
Sitronix ST7571 is a 4bit gray scale dot matrix LCD controller. The controller has a SPI, I2C and 8bit parallel interface, this is for the I2C interface only. Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com> --- .../bindings/display/sitronix,st7571-i2c.yaml | 71 ++++++++++++++++++++++ 1 file changed, 71 insertions(+)