Re: [PATCH v2 1/2] dt-bindings: display: Add SSD133x OLED controllers
Krzysztof Kozlowski 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 LINTDocumentation/devicetree/bindings CHKDT Documentation/devicetree/bindings/processed-schema.json SCHEMA Documentation/devicetree/bindings/processed-schema.json DTEXDocumentation/devicetree/bindings/display/solomon,ssd133x.example.dts DTC_CHK Documentation/devicetree/bindings/display/solomon,ssd133x.example.dtb > Best regards, > Krzysztof > -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH v2 1/2] dt-bindings: display: Add SSD133x OLED controllers
On 19/12/2023 12:20, Javier Martinez Canillas wrote: > Conor Dooley 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 >> >> +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
Re: [PATCH v2 1/2] dt-bindings: display: Add SSD133x OLED controllers
Conor Dooley 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
>
> +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:
LINTDocumentation/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
DTEXDocumentation/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
Re: [PATCH v2 1/2] dt-bindings: display: Add SSD133x OLED controllers
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 > --- > > 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 ..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 > + > +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 +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? signature.asc Description: PGP signature
[PATCH v2 1/2] dt-bindings: display: Add SSD133x OLED controllers
Add a Device Tree binding schema for the OLED panels based on the
Solomon SSD133x family of controllers.
Signed-off-by: Javier Martinez Canillas
---
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 ..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
+
+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 = <1000>;
+};
+};
--
2.43.0
Re: [PATCH v2 1/2] dt-bindings: display: Add SSD133x OLED controllers
Conor Dooley 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
>
> +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.
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
