Add a Device Tree binding schema for the OLED panels based on the Solomon
SSD132x family of controllers.
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---
.../bindings/display/solomon,ssd132x.yaml | 116 ++++++++++++++++++
1 file changed, 116 insertions(+)
create mode 100644 Documentation/devicetree/bindings/display/solomon,ssd132x.yaml
diff --git a/Documentation/devicetree/bindings/display/solomon,ssd132x.yaml b/Documentation/devicetree/bindings/display/solomon,ssd132x.yaml
new file mode 100644
index 000000000000..b64904703a3a
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/solomon,ssd132x.yaml
@@ -0,0 +1,116 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/solomon,ssd132x.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Solomon SSD132x OLED Controllers
+
+maintainers:
+ - Javier Martinez Canillas <javierm@redhat.com>
+
+properties:
+ compatible:
+ oneOf:
+ - enum:
+ - solomon,ssd1322
+ - solomon,ssd1325
+ - solomon,ssd1327
+
+ reg:
+ maxItems: 1
+
+ reset-gpios:
+ maxItems: 1
+
+ # Only required for SPI
+ dc-gpios:
+ description:
+ GPIO connected to the controller's D/C# (Data/Command) pin,
+ that is needed for 4-wire SPI to tell the controller if the
+ data sent is for a command register or the display data RAM
+ maxItems: 1
+
+ solomon,height:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Height in pixel of the screen driven by the controller.
+ The default value is controller-dependent.
+
+ solomon,width:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Width in pixel of the screen driven by the controller.
+ The default value is controller-dependent.
+
+required:
+ - compatible
+ - reg
+
+allOf:
+ - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: solomon,ssd1322
+ then:
+ properties:
+ width:
+ default: 480
+ height:
+ default: 128
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: solomon,ssd1325
+ then:
+ properties:
+ width:
+ default: 128
+ height:
+ default: 80
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: solomon,ssd1327
+ then:
+ properties:
+ width:
+ default: 128
+ height:
+ default: 128
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ssd1327_i2c: oled@3c {
+ compatible = "solomon,ssd1327";
+ reg = <0x3c>;
+ reset-gpios = <&gpio2 7>;
+ };
+
+ };
+ - |
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ssd1327_spi: oled@0 {
+ compatible = "solomon,ssd1327";
+ reg = <0x0>;
+ reset-gpios = <&gpio2 7>;
+ dc-gpios = <&gpio2 8>;
+ spi-max-frequency = <10000000>;
+ };
+ };
--
2.41.0On Mon, Oct 09, 2023 at 08:34:22PM +0200, Javier Martinez Canillas wrote: > Add a Device Tree binding schema for the OLED panels based on the Solomon > SSD132x family of controllers. Looks like the same binding as solomon,ssd1307fb.yaml. Why a different binding? Why does that binding need a slew of custom properties and here you don't? > > Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> > --- > > .../bindings/display/solomon,ssd132x.yaml | 116 ++++++++++++++++++ > 1 file changed, 116 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/solomon,ssd132x.yaml > > diff --git a/Documentation/devicetree/bindings/display/solomon,ssd132x.yaml b/Documentation/devicetree/bindings/display/solomon,ssd132x.yaml > new file mode 100644 > index 000000000000..b64904703a3a > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/solomon,ssd132x.yaml > @@ -0,0 +1,116 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/solomon,ssd132x.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Solomon SSD132x OLED Controllers > + > +maintainers: > + - Javier Martinez Canillas <javierm@redhat.com> > + > +properties: > + compatible: > + oneOf: > + - enum: > + - solomon,ssd1322 > + - solomon,ssd1325 > + - solomon,ssd1327 > + > + reg: > + maxItems: 1 > + > + reset-gpios: > + maxItems: 1 > + > + # Only required for SPI > + dc-gpios: > + description: > + GPIO connected to the controller's D/C# (Data/Command) pin, > + that is needed for 4-wire SPI to tell the controller if the > + data sent is for a command register or the display data RAM > + maxItems: 1 > + > + solomon,height: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + Height in pixel of the screen driven by the controller. > + The default value is controller-dependent. > + > + solomon,width: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + Width in pixel of the screen driven by the controller. > + The default value is controller-dependent. Don't define the same properties twice. Either share the binding or split out the common properties into their own schema file. Rob
Rob Herring <robh@kernel.org> writes: Hello Rob, Thanks a lot for your feedback. > On Mon, Oct 09, 2023 at 08:34:22PM +0200, Javier Martinez Canillas wrote: >> Add a Device Tree binding schema for the OLED panels based on the Solomon >> SSD132x family of controllers. > > Looks like the same binding as solomon,ssd1307fb.yaml. Why a different > binding? Why does that binding need a slew of custom properties and here > you don't? > It's a sub-set of it. Because only the minimum required properties are defined. But also, is a clean slate schema because the old ssd1307fb fbdev driver only supports the Solomon SSD130x family, so there is no need to follow the existing solomon,ssd1307fb.yaml nor the need for backward compat. [...] >> + solomon,height: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + description: >> + Height in pixel of the screen driven by the controller. >> + The default value is controller-dependent. >> + >> + solomon,width: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + description: >> + Width in pixel of the screen driven by the controller. >> + The default value is controller-dependent. > > Don't define the same properties twice. Either share the binding or > split out the common properties into their own schema file. > Agreed. I'll do that in v2. > Rob > -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Javier Martinez Canillas <javierm@redhat.com> writes: > Rob Herring <robh@kernel.org> writes: > > Hello Rob, > > Thanks a lot for your feedback. > >> On Mon, Oct 09, 2023 at 08:34:22PM +0200, Javier Martinez Canillas wrote: >>> Add a Device Tree binding schema for the OLED panels based on the Solomon >>> SSD132x family of controllers. >> >> Looks like the same binding as solomon,ssd1307fb.yaml. Why a different >> binding? Why does that binding need a slew of custom properties and here >> you don't? >> > > It's a sub-set of it. Because only the minimum required properties are > defined. But also, is a clean slate schema because the old ssd1307fb fbdev > driver only supports the Solomon SSD130x family, so there is no need to > follow the existing solomon,ssd1307fb.yaml nor the need for backward compat. > I think this answer was a little sparse, let me elaborate a bit. The Solomon display controllers are quite flexible, so that could be used with different OLED panels. And the ssd1307fb binding schema defines a set of properties that are almost a 1:1 mapping from properties to the configuration registers. This makes the driver to support most SSD130x controller + panel configurations but at the expense of making the binding more complicated (a slew of custom propertie as you pointed out). Now, as mentioned this is support for greyscale Solomon controllers (the old fbdev driver only supports monochrome controllers) so we don't care about DT backward compatibility. I decided for now to keep the binding at a minimum and be more opinionated in the driver with having what I think are sane defaults for most of the config registers. If there is a need to expose any of this configuration as DT properties, then we can later added it share some of the existing solomon,ssd1307fb.yaml custom properties and move them to a common schema. We can always change the driver in the future anyways, but we can't do the same with the DT binding schema since that is considered an ABI. -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Hey,
On Mon, Oct 09, 2023 at 08:34:22PM +0200, Javier Martinez Canillas wrote:
> Add a Device Tree binding schema for the OLED panels based on the Solomon
> SSD132x family of controllers.
>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
>
> .../bindings/display/solomon,ssd132x.yaml | 116 ++++++++++++++++++
> 1 file changed, 116 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/solomon,ssd132x.yaml
>
> diff --git a/Documentation/devicetree/bindings/display/solomon,ssd132x.yaml b/Documentation/devicetree/bindings/display/solomon,ssd132x.yaml
> new file mode 100644
> index 000000000000..b64904703a3a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/solomon,ssd132x.yaml
> @@ -0,0 +1,116 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/solomon,ssd132x.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Solomon SSD132x OLED Controllers
> +
> +maintainers:
> + - Javier Martinez Canillas <javierm@redhat.com>
> +
> +properties:
> + compatible:
> + oneOf:
> + - enum:
> + - solomon,ssd1322
> + - solomon,ssd1325
> + - solomon,ssd1327
You don't need the oneOf here here as there is only the enum as a
possible item.
I didn't get anything else in the series, I have to ask - are these
controllers not compatible with eachother?
> +
> + reg:
> + maxItems: 1
> +
> + reset-gpios:
> + maxItems: 1
> +
> + # Only required for SPI
> + dc-gpios:
> + description:
> + GPIO connected to the controller's D/C# (Data/Command) pin,
> + that is needed for 4-wire SPI to tell the controller if the
> + data sent is for a command register or the display data RAM
> + maxItems: 1
> +
> + solomon,height:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + Height in pixel of the screen driven by the controller.
> + The default value is controller-dependent.
You probably know better than me, operating in drm stuff, but are there
really no generic properties for the weidth/height of a display?
> +
> + solomon,width:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + Width in pixel of the screen driven by the controller.
> + The default value is controller-dependent.
> +
> +required:
> + - compatible
> + - reg
> +
> +allOf:
> + - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: solomon,ssd1322
> + then:
> + properties:
> + width:
> + default: 480
> + height:
> + default: 128
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: solomon,ssd1325
> + then:
> + properties:
> + width:
> + default: 128
> + height:
> + default: 80
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: solomon,ssd1327
> + then:
> + properties:
> + width:
> + default: 128
> + height:
> + default: 128
Unless you did it like this for clarity, 2 of these have the same
default width and 2 have the same default height. You could cut this
down to a pair of if/then/else on that basis AFAICT.
:wq
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ssd1327_i2c: oled@3c {
This label is unused as far as I can tell. Ditto below.
Cheers,
Conor.
> + compatible = "solomon,ssd1327";
> + reg = <0x3c>;
> + reset-gpios = <&gpio2 7>;
> + };
> +
> + };
> + - |
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ssd1327_spi: oled@0 {
> + compatible = "solomon,ssd1327";
> + reg = <0x0>;
> + reset-gpios = <&gpio2 7>;
> + dc-gpios = <&gpio2 8>;
> + spi-max-frequency = <10000000>;
> + };
> + };
> --
> 2.41.0
>
>
Conor Dooley <conor@kernel.org> writes:
Hello Conor,
Thanks a lot for your feedback.
> Hey,
>
> On Mon, Oct 09, 2023 at 08:34:22PM +0200, Javier Martinez Canillas wrote:
[...]
>> +properties:
>> + compatible:
>> + oneOf:
>> + - enum:
>> + - solomon,ssd1322
>> + - solomon,ssd1325
>> + - solomon,ssd1327
>
> You don't need the oneOf here here as there is only the enum as a
> possible item.
Indeed. I'll fix that in v2.
> I didn't get anything else in the series, I have to ask - are these
> controllers not compatible with eachother?
>
They are not, basically the difference is in the default width and height
for each controller. That's why the width and height fields are optional.
But other than the default resolution, yes the controllers are very much
the same.
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + reset-gpios:
>> + maxItems: 1
>> +
>> + # Only required for SPI
>> + dc-gpios:
>> + description:
>> + GPIO connected to the controller's D/C# (Data/Command) pin,
>> + that is needed for 4-wire SPI to tell the controller if the
>> + data sent is for a command register or the display data RAM
>> + maxItems: 1
>> +
>> + solomon,height:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description:
>> + Height in pixel of the screen driven by the controller.
>> + The default value is controller-dependent.
>
> You probably know better than me, operating in drm stuff, but are there
> really no generic properties for the weidth/height of a display?
>
There are some common properties, such as the width-mm and height-mm for
the panel-common:
Documentation/devicetree/bindings/display/panel/panel-common.yaml
But those are to describe the physical area expressed in millimeters and
the Solomon drivers (the old ssd1307fb fbdev driver and the new ssd130x
DRM driver for backward compatibility with existing DTB) express the width
and height in pixels.
That's why are Solomon controller specific properties "solomon,width" and
"solomon,height".
[...]
>> + then:
>> + properties:
>> + width:
>> + default: 128
>> + height:
>> + default: 128
>
> Unless you did it like this for clarity, 2 of these have the same
> default width and 2 have the same default height. You could cut this
> down to a pair of if/then/else on that basis AFAICT.
> :wq
>
Yes, this was done like that for clarity. Because is easier for someone
reading the DT binding schema to reason about resolution (width,height)
for a given SSD132x controller, rather than following the if/else logic.
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> + - |
>> + i2c {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + ssd1327_i2c: oled@3c {
>
> This label is unused as far as I can tell. Ditto below.
>
Right, I'll drop those too.
> Cheers,
> Conor.
>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
On Wed, Oct 11, 2023 at 08:34:29AM +0200, Javier Martinez Canillas wrote: > Conor Dooley <conor@kernel.org> writes: > >> + # Only required for SPI > >> + dc-gpios: > >> + description: > >> + GPIO connected to the controller's D/C# (Data/Command) pin, > >> + that is needed for 4-wire SPI to tell the controller if the > >> + data sent is for a command register or the display data RAM > >> + maxItems: 1 > >> + > >> + solomon,height: > >> + $ref: /schemas/types.yaml#/definitions/uint32 > >> + description: > >> + Height in pixel of the screen driven by the controller. > >> + The default value is controller-dependent. > > > > You probably know better than me, operating in drm stuff, but are there > > really no generic properties for the weidth/height of a display? > > > > There are some common properties, such as the width-mm and height-mm for > the panel-common: > > Documentation/devicetree/bindings/display/panel/panel-common.yaml > > But those are to describe the physical area expressed in millimeters and > the Solomon drivers (the old ssd1307fb fbdev driver and the new ssd130x > DRM driver for backward compatibility with existing DTB) express the width > and height in pixels. > > That's why are Solomon controller specific properties "solomon,width" and > "solomon,height". Okay. Thanks for the explanation.
© 2016 - 2026 Red Hat, Inc.