Re: [PATCH 8/8] dt-bindings: display: Add SSD132x OLED controllers

[Javier Martinez Canillas <javierm@redhat.com>]

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