| Message ID | 20250404-st7571-v2-1-4c78aab9cd5a@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for Sitronix ST7571 LCD controller | expand |
On Fri, Apr 04, 2025 at 03:50:32PM +0200, Marcus Folkesson wrote: > Sitronix ST7571 is a 4bit gray scale dot matrix LCD controller. > > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com> > --- > .../bindings/display/sitronix,st7571.yaml | 73 ++++++++++++++++++++++ > 1 file changed, 73 insertions(+) > > diff --git a/Documentation/devicetree/bindings/display/sitronix,st7571.yaml b/Documentation/devicetree/bindings/display/sitronix,st7571.yaml > new file mode 100644 > index 0000000000000000000000000000000000000000..11575b820c59c5ada427fbb6b015c331215c8db6 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/sitronix,st7571.yaml > @@ -0,0 +1,73 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/sitronix,st7571.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Sitronix ST7571 Display Panels > + > +maintainers: > + - Marcus Folkesson <marcus.folkesson@gmail.com> > + > +description: > + This binding is for display panels using a Sitronix ST7571 controller in I2C > + mode. > + > +allOf: > + - $ref: panel/panel-common.yaml# > + > +properties: > + compatible: > + const: sitronix,st7571 > + > + reg: true You need to constrain this, so maxItems: 1. Otherwise, seems okay.
On 04/04/2025 15:50, Marcus Folkesson wrote: > Sitronix ST7571 is a 4bit gray scale dot matrix LCD controller. > > Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com> > --- > .../bindings/display/sitronix,st7571.yaml | 73 ++++++++++++++++++++++ > 1 file changed, 73 insertions(+) > > diff --git a/Documentation/devicetree/bindings/display/sitronix,st7571.yaml b/Documentation/devicetree/bindings/display/sitronix,st7571.yaml > new file mode 100644 > index 0000000000000000000000000000000000000000..11575b820c59c5ada427fbb6b015c331215c8db6 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/sitronix,st7571.yaml > @@ -0,0 +1,73 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/sitronix,st7571.yaml# Why isn't this in panels directory? > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Sitronix ST7571 Display Panels > + > +maintainers: > + - Marcus Folkesson <marcus.folkesson@gmail.com> > + > +description: > + This binding is for display panels using a Sitronix ST7571 controller in I2C Do not explain for a binding that it is a binding. Redundant. Instead describe the hardware. > + mode. > + > +allOf: > + - $ref: panel/panel-common.yaml# > + > +properties: > + compatible: > + const: sitronix,st7571 > + > + reg: true > + reset-gpios: true > + width-mm: true > + height-mm: true > + > + panel-timing: > + $ref: panel/panel-timing.yaml# Drop, already part of panel. > + description: | > + The panel-timing node specifies the display resolution and timing > + parameters. The hactive and vactive properties are mandatory. > + The vback-porch property specifies the start line of the display. > + The other properties should be set to zero. Drop description as well. > + > +required: > + - compatible > + - reg > + - reset-gpios > + - width-mm > + - height-mm > + - panel-timing > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/gpio/gpio.h> > + > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + display@3f { Not much improved. How is this called in every other binding? panel. > + compatible = "sitronix,st7571"; Messed indentation. > + reg = <0x3f>; > + reset-gpios = <&gpio0 3 GPIO_ACTIVE_LOW>; > + width-mm = <37>; > + height-mm = <27>; > + > + panel-timing { And here is even more confusing. Best regards, Krzysztof
On 04/04/2025 19:30, Krzysztof Kozlowski wrote: >> + >> +examples: >> + - | >> + #include <dt-bindings/gpio/gpio.h> >> + >> + i2c { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + display@3f { > > Not much improved. How is this called in every other binding? panel. Hmmm, unless this is not a panel, but it looks like a panel and description partially suggests it. Other sitronix devices are split between these two, but OTOH your driver is more complex than just simple panel. Your commit msg is one sentence and binding description is basically non-existing, so not sure how to help. You need to describe the hardware so people understand what this device is. Best regards, Krzysztof
Hi Krzysztof, On Fri, Apr 04, 2025 at 07:36:12PM +0200, Krzysztof Kozlowski wrote: > On 04/04/2025 19:30, Krzysztof Kozlowski wrote: > >> + > >> +examples: > >> + - | > >> + #include <dt-bindings/gpio/gpio.h> > >> + > >> + i2c { > >> + #address-cells = <1>; > >> + #size-cells = <0>; > >> + > >> + display@3f { > > > > Not much improved. How is this called in every other binding? panel. > > Hmmm, unless this is not a panel, but it looks like a panel and > description partially suggests it. Other sitronix devices are split > between these two, but OTOH your driver is more complex than just simple > panel. I've counted this as a display, but the border is not crystal clear, and, as you say, other Sitronix devices are split between the two. It is a controller/driver for a LCD panel. > > Your commit msg is one sentence and binding description is basically > non-existing, so not sure how to help. You need to describe the hardware > so people understand what this device is. I've prepared this description for the next version of the patch: description: Sitronix ST7571 is a driver and controller for up to 4-level gray scale dot-matrix LCD panels. It drives 128 segment outputs and 128+1 common outputs. It provides several system interfaces like SPI, I2C and 8-bit parallel bus. But still, it is not obvious if I should move it to panel or not. > > Best regards, > Krzysztof Best regards, Marcus Folkesson
diff --git a/Documentation/devicetree/bindings/display/sitronix,st7571.yaml b/Documentation/devicetree/bindings/display/sitronix,st7571.yaml new file mode 100644 index 0000000000000000000000000000000000000000..11575b820c59c5ada427fbb6b015c331215c8db6 --- /dev/null +++ b/Documentation/devicetree/bindings/display/sitronix,st7571.yaml @@ -0,0 +1,73 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/sitronix,st7571.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Sitronix ST7571 Display Panels + +maintainers: + - Marcus Folkesson <marcus.folkesson@gmail.com> + +description: + This binding is for display panels using a Sitronix ST7571 controller in I2C + mode. + +allOf: + - $ref: panel/panel-common.yaml# + +properties: + compatible: + const: sitronix,st7571 + + reg: true + reset-gpios: true + width-mm: true + height-mm: true + + panel-timing: + $ref: panel/panel-timing.yaml# + description: | + The panel-timing node specifies the display resolution and timing + parameters. The hactive and vactive properties are mandatory. + The vback-porch property specifies the start line of the display. + The other properties should be set to zero. + +required: + - compatible + - reg + - reset-gpios + - width-mm + - height-mm + - panel-timing + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + + i2c { + #address-cells = <1>; + #size-cells = <0>; + + display@3f { + compatible = "sitronix,st7571"; + reg = <0x3f>; + reset-gpios = <&gpio0 3 GPIO_ACTIVE_LOW>; + width-mm = <37>; + height-mm = <27>; + + panel-timing { + hactive = <128>; + vactive = <96>; + hback-porch = <0>; + vback-porch = <0>; + clock-frequency = <0>; + hfront-porch = <0>; + hsync-len = <0>; + vfront-porch = <0>; + vsync-len = <0>; + }; + }; + };
Sitronix ST7571 is a 4bit gray scale dot matrix LCD controller. Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com> --- .../bindings/display/sitronix,st7571.yaml | 73 ++++++++++++++++++++++ 1 file changed, 73 insertions(+)