Re: [PATCH 1/3] dt-bindings: display: add Chipwealth CH1115 OLED controller

[email protected]
Newsgroups org.freedesktop.lists.dri-devel,org.kernel.vger.linux-devicetree
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The device tree schema is structurally unsatisfiable and introduces hardware-specific properties without vendor prefixes. `first-page` is mandated by the `required` list but is not defined in the `properties` block. Because `additionalProperties: false` is used, any device tree node including `first-page` is rejected, while any node omitting it fails the `required` check. Additionally, `contrast` is used in the `examples` but missing from `properties`. Finally, `invert`, `first-page`, and `contrast` are custom properties that must include a vendor prefix (e.g., `chipwealth,invert`) to avoid polluting the global namespace.
--

--- Patch [1]: [PATCH 1/3] dt-bindings: display: add Chipwealth CH1115 OLED controller ---
commit 77e215ca91e888007e7e74fb225f1cb55be392a1
Author: Nicolás Antinori <[email protected]>

dt-bindings: display: add Chipwealth CH1115 OLED controller

This commit adds the device tree binding for the Chipwealth CH1115 monochrome
dot matrix OLED panel controller.

> diff --git a/Documentation/devicetree/bindings/display/chipwealth,ch1115.yaml b/Documentation/devicetree/bindings/display/chipwealth,ch1115.yaml
> new file mode 100644
> index 0000000000000..29d55da85d219
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/chipwealth,ch1115.yaml
[ ... ]
> +properties:
> +  compatible:
> +    const: chipwealth,ch1115
> +
> +  reg:
> +    maxItems: 1
> +
> +  invert:
> +    type: boolean
> +    description:
> +      Display pixels are inverted, i.e. 0 is white and 1 is black.

[Severity: Medium]
Should this property have a vendor prefix (e.g., chipwealth,invert)? Custom
hardware-specific properties typically require vendor prefixes to avoid
polluting the global namespace.

> +
> +  width-mm: true
> +  height-mm: true
> +  panel-timing: true
> +
> +required:
> +  - compatible
> +  - reg
> +  - width-mm
> +  - height-mm
> +  - panel-timing
> +  - first-page
> +
> +additionalProperties: false

[Severity: Medium]
Is first-page missing from the properties block? Because it is explicitly
mandated by the required list but not defined in properties,
additionalProperties: false will forbid it, making the schema unsatisfiable
for any device tree node.

> +
> +examples:
> +  - |
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      status = "okay";
> +
> +      display@3c {
> +        compatible = "chipwealth,ch1115";
> +        reg = <0x3c>;
> +        width-mm = <12>;
> +        height-mm = <8>;
> +        contrast = <127>;
> +        first-page = <1>;

[Severity: Medium]
Are contrast and first-page intended to be defined in this schema? They are
used here in the example but missing from the properties block, which will
cause dt_binding_check to fail. As with invert, do these custom properties
need vendor prefixes?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.