| Message ID | 20231218132045.2066576-2-javierm@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drm/solomon: Add support for the SSD133x controller family | expand |
On Mon, Dec 18, 2023 at 02:20:35PM +0100, Javier Martinez Canillas wrote: > Add a Device Tree binding schema for the OLED panels based on the > Solomon SSD133x family of controllers. > > Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> > --- > > Changes in v2: > - Unconditionally set the width and height constraints (Conor Dooley). > - Fix indentation in the DTS examples (Krzysztof Kozlowski). > > .../bindings/display/solomon,ssd133x.yaml | 57 +++++++++++++++++++ > 1 file changed, 57 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/solomon,ssd133x.yaml > > diff --git a/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml b/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml > new file mode 100644 > index 000000000000..8feee9eef0fd > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml > @@ -0,0 +1,57 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/solomon,ssd133x.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Solomon SSD133x OLED Display Controllers > + > +maintainers: > + - Javier Martinez Canillas <javierm@redhat.com> > + > +properties: > + compatible: > + enum: > + - solomon,ssd1331 > + > +required: > + - compatible > + - reg > + > +allOf: > + - $ref: solomon,ssd-common.yaml# > + > + - properties: > + width: > + default: 96 > + height: > + default: 64 diff --git a/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml b/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml index 8feee9eef0fd..ffc939c782eb 100644 --- a/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml +++ b/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml @@ -9,24 +9,24 @@ title: Solomon SSD133x OLED Display Controllers maintainers: - Javier Martinez Canillas <javierm@redhat.com> +allOf: + - $ref: solomon,ssd-common.yaml# + properties: compatible: enum: - solomon,ssd1331 + width: + default: 96 + + height: + default: 64 + required: - compatible - reg -allOf: - - $ref: solomon,ssd-common.yaml# - - - properties: - width: - default: 96 - height: - default: 64 - unevaluatedProperties: false examples: The properties stuff doesn't need to be in the allOf. Although, I took the opportunity to look at ssd-common.yaml. How do the height/width here differ from the vendor prefixed versions in that file?
Conor Dooley <conor@kernel.org> writes: Hello Conor, Thanks a lot for your feedback. > On Mon, Dec 18, 2023 at 02:20:35PM +0100, Javier Martinez Canillas wrote: [...] >> + >> + - properties: >> + width: >> + default: 96 >> + height: >> + default: 64 > > diff --git a/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml b/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml > index 8feee9eef0fd..ffc939c782eb 100644 > --- a/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml > +++ b/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml > @@ -9,24 +9,24 @@ title: Solomon SSD133x OLED Display Controllers > maintainers: > - Javier Martinez Canillas <javierm@redhat.com> > > +allOf: > + - $ref: solomon,ssd-common.yaml# > + > properties: > compatible: > enum: > - solomon,ssd1331 > > + width: > + default: 96 > + > + height: > + default: 64 > + > required: > - compatible > - reg > > -allOf: > - - $ref: solomon,ssd-common.yaml# > - > - - properties: > - width: > - default: 96 > - height: > - default: 64 > - > unevaluatedProperties: false > > examples: > > The properties stuff doesn't need to be in the allOf. Although, I took Ok. > the opportunity to look at ssd-common.yaml. How do the height/width here > differ from the vendor prefixed versions in that file? Oh! That is an error in the schema that I introduced when adding the binding for the SSD132x family in commit 2d23e7d6bacb ("dt-bindings: display: Add SSD132x OLED controllers"), and I just copied it to this binding as well making the same mistake... I'll fix that with a preparatory patch to use "solomon,{width,height}" everywhere in v3 and also include your suggested changes for this patch.
Conor Dooley <conor@kernel.org> writes: Hello Conor, > On Mon, Dec 18, 2023 at 02:20:35PM +0100, Javier Martinez Canillas wrote: [...] >> +allOf: >> + - $ref: solomon,ssd-common.yaml# >> + >> + - properties: >> + width: >> + default: 96 >> + height: >> + default: 64 > > diff --git a/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml b/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml > index 8feee9eef0fd..ffc939c782eb 100644 > --- a/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml > +++ b/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml > @@ -9,24 +9,24 @@ title: Solomon SSD133x OLED Display Controllers > maintainers: > - Javier Martinez Canillas <javierm@redhat.com> > > +allOf: > + - $ref: solomon,ssd-common.yaml# > + This part worked correctly... > properties: > compatible: > enum: > - solomon,ssd1331 > > + width: > + default: 96 > + > + height: > + default: 64 > + ...but when trying move the default for the "solomon,width" and "solomon,height" to the properties section, make dt_binding_check complains as follows: LINT Documentation/devicetree/bindings CHKDT Documentation/devicetree/bindings/processed-schema.json /home/javier/devel/linux/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml: properties:solomon,height: 'oneOf' conditional failed, one must be fixed: 'type' is a required property hint: A vendor boolean property can use "type: boolean" 'description' is a required property hint: A vendor boolean property can use "type: boolean" Additional properties are not allowed ('default' was unexpected) hint: A vendor boolean property can use "type: boolean" /home/javier/devel/linux/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml: properties:solomon,height: 'oneOf' conditional failed, one must be fixed: 'enum' is a required property 'const' is a required property hint: A vendor string property with exact values has an implicit type from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml# /home/javier/devel/linux/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml: properties:solomon,height: 'oneOf' conditional failed, one must be fixed: '$ref' is a required property 'allOf' is a required property hint: A vendor property needs a $ref to types.yaml from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml# hint: Vendor specific properties must have a type and description unless they have a defined, common suffix. from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml# /home/javier/devel/linux/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml: properties:solomon,width: 'oneOf' conditional failed, one must be fixed: 'type' is a required property hint: A vendor boolean property can use "type: boolean" 'description' is a required property hint: A vendor boolean property can use "type: boolean" Additional properties are not allowed ('default' was unexpected) hint: A vendor boolean property can use "type: boolean" /home/javier/devel/linux/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml: properties:solomon,width: 'oneOf' conditional failed, one must be fixed: 'enum' is a required property 'const' is a required property hint: A vendor string property with exact values has an implicit type from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml# /home/javier/devel/linux/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml: properties:solomon,width: 'oneOf' conditional failed, one must be fixed: '$ref' is a required property 'allOf' is a required property hint: A vendor property needs a $ref to types.yaml from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml# hint: Vendor specific properties must have a type and description unless they have a defined, common suffix. from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml# SCHEMA Documentation/devicetree/bindings/processed-schema.json DTEX Documentation/devicetree/bindings/display/solomon,ssd133x.example.dts /home/javier/.local/bin/dt-extract-example:75: SyntaxWarning: invalid escape sequence '\s' root_node = re.search('/\s*{', ex) /home/javier/.local/bin/dt-extract-example:79: SyntaxWarning: invalid escape sequence '\s' int_val = re.search('\sinterrupts\s*=\s*<([0-9a-zA-Z |()_]+)>', ex).group(1) DTC_CHK Documentation/devicetree/bindings/display/solomon,ssd133x.example.dtb The warning goes away if I follow the hints and add a type and description to the properties, i.e: diff --git a/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml b/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml index 880c71fdec68..0f4d9ca7456b 100644 --- a/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml +++ b/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml @@ -17,6 +17,20 @@ properties: enum: - solomon,ssd1331 + 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. + default: 96 + + 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. + default: 64 + required: - compatible - reg But that would duplicate information that is already present in the included solomon,ssd-common.yaml schema. Do you know what is the proper way to do this? Or maybe should I just drop the default values for the width and height properties? I just think that is good information to have in the schema.
On 19/12/2023 12:20, Javier Martinez Canillas wrote: > Conor Dooley <conor@kernel.org> writes: > > Hello Conor, > >> On Mon, Dec 18, 2023 at 02:20:35PM +0100, Javier Martinez Canillas wrote: > > [...] > >>> +allOf: >>> + - $ref: solomon,ssd-common.yaml# >>> + >>> + - properties: >>> + width: >>> + default: 96 >>> + height: >>> + default: 64 >> >> diff --git a/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml b/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml >> index 8feee9eef0fd..ffc939c782eb 100644 >> --- a/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml >> +++ b/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml >> @@ -9,24 +9,24 @@ title: Solomon SSD133x OLED Display Controllers >> maintainers: >> - Javier Martinez Canillas <javierm@redhat.com> >> >> +allOf: >> + - $ref: solomon,ssd-common.yaml# >> + > > This part worked correctly... > >> properties: >> compatible: >> enum: >> - solomon,ssd1331 >> >> + width: >> + default: 96 >> + >> + height: >> + default: 64 Which also looks wrong on its own. Where is the definition of these properties? IOW, where do they come from? >> + > > ...but when trying move the default for the "solomon,width" and > "solomon,height" to the properties section, make dt_binding_check > complains as follows: Worked for me. ... > DTC_CHK Documentation/devicetree/bindings/display/solomon,ssd133x.example.dtb > > The warning goes away if I follow the hints and add a type and description > to the properties, i.e: Hm, I wonder what's different in your case. I assume you run the latest dtschema. > > diff --git a/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml b/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml > index 880c71fdec68..0f4d9ca7456b 100644 > --- a/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml > +++ b/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml > @@ -17,6 +17,20 @@ properties: > enum: > - solomon,ssd1331 > > + 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. > + default: 96 > + > + 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. > + default: 64 > + > required: > - compatible > - reg > > But that would duplicate information that is already present in the > included solomon,ssd-common.yaml schema. Do you know what is the proper > way to do this? Works for me, so please paste somewhere proper diff so we can compare. Best regards, Krzysztof
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> writes: Hello Krzysztof, > On 19/12/2023 12:20, Javier Martinez Canillas wrote: [...] >>>> +allOf: >>>> + - $ref: solomon,ssd-common.yaml# >>>> + [...] >>> >>> + width: >>> + default: 96 >>> + >>> + height: >>> + default: 64 > > Which also looks wrong on its own. Where is the definition of these Yes, I already discussed this with Conor and mentioned to him that is a typo but already fixed it locally and I'm testing with the correct ones. > properties? IOW, where do they come from? > The "solomon,width" and "solomon,height" properties are defined in the solomon,ssd-common.yaml binding schema file that is referenced. >>> + >> >> ...but when trying move the default for the "solomon,width" and >> "solomon,height" to the properties section, make dt_binding_check >> complains as follows: > > Worked for me. > Oh, that's good to know. I wonder what's the difference... > ... > >> DTC_CHK Documentation/devicetree/bindings/display/solomon,ssd133x.example.dtb >> >> The warning goes away if I follow the hints and add a type and description >> to the properties, i.e: > > Hm, I wonder what's different in your case. I assume you run the latest > dtschema. > Not the latest but had a recent one. I've updated it, so I do now :) $ pip list | grep dtschema dtschema 2023.9 $ pip install --upgrade dtschema $ pip list | grep dtschema dtschema 2023.11 [...] >> But that would duplicate information that is already present in the >> included solomon,ssd-common.yaml schema. Do you know what is the proper >> way to do this? > > Works for me, so please paste somewhere proper diff so we can compare. > With the latest dtschema version it works indeed. Thanks for the pointer! $ make W=1 dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/display/solomon,ssd133x.yaml LINT Documentation/devicetree/bindings CHKDT Documentation/devicetree/bindings/processed-schema.json SCHEMA Documentation/devicetree/bindings/processed-schema.json DTEX Documentation/devicetree/bindings/display/solomon,ssd133x.example.dts DTC_CHK Documentation/devicetree/bindings/display/solomon,ssd133x.example.dtb > Best regards, > Krzysztof >
diff --git a/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml b/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml new file mode 100644 index 000000000000..8feee9eef0fd --- /dev/null +++ b/Documentation/devicetree/bindings/display/solomon,ssd133x.yaml @@ -0,0 +1,57 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/solomon,ssd133x.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Solomon SSD133x OLED Display Controllers + +maintainers: + - Javier Martinez Canillas <javierm@redhat.com> + +properties: + compatible: + enum: + - solomon,ssd1331 + +required: + - compatible + - reg + +allOf: + - $ref: solomon,ssd-common.yaml# + + - properties: + width: + default: 96 + height: + default: 64 + +unevaluatedProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + oled@3c { + compatible = "solomon,ssd1331"; + reg = <0x3c>; + reset-gpios = <&gpio2 7>; + }; + + }; + - | + spi { + #address-cells = <1>; + #size-cells = <0>; + + oled@0 { + compatible = "solomon,ssd1331"; + reg = <0x0>; + reset-gpios = <&gpio2 7>; + dc-gpios = <&gpio2 8>; + spi-max-frequency = <10000000>; + }; + };
Add a Device Tree binding schema for the OLED panels based on the Solomon SSD133x family of controllers. Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> --- Changes in v2: - Unconditionally set the width and height constraints (Conor Dooley). - Fix indentation in the DTS examples (Krzysztof Kozlowski). .../bindings/display/solomon,ssd133x.yaml | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/solomon,ssd133x.yaml