Re: [PATCH v2 1/5] dt-bindings: display: Add Apple pre-DCP display controller bindings

[Krzysztof Kozlowski <krzk@kernel.org>]

On 26/11/2024 18:00, Sasha Finkelstein wrote:
> On Tue, 26 Nov 2024 at 17:46, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>> +allOf:
>>> +  - $ref: dsi-controller.yaml#
> ...
>>> +patternProperties:
>>> +  "^panel@[0-3]$": true
>>
>> These look unusual. Is this a DSI controller? If so, then reference
>> dsi-controller. See other bindings how this is done.
> 
> This is a DSI controller, as referenced above. Those properties are from
> dsi-controller.yaml

Ahh, indeed, I missed that and focused on the additionalProperties.
Instead use unevaluatedProperties: false and drop all properties already
in dsi-controller.yaml.

> 
>>> +properties:
>>> +  compatible:
>>> +    items:
>>> +      - enum:
>>> +          - apple,j293-summit
>>> +          - apple,j493-summit
>>> +      - const: apple,summit
>>
>> Summit tells me nothing - no description, title repeats it, so I suggest
>> using device specific compatible.
> 
> The j293/j493 are the device-specific compatibles, those are chassis names
> for the specific laptops the panel is present in.

This does not address my comment. Use specific compatibles as fallback,
just like we recommend for every device. This should not be different.
If you do not know the hardware details, using generic is even less
appropriate.

> 
> 
>> No, these cannot be true without definition. Again, please open existing
>> bindings and use them as example. You probably miss here some reference,
>> but max-brightness for panel is a bit confusing. I asked already and did
>> not get answer: isn't this backlight property? What is this device -
>> backlight or panel? If panel, then what bus?
> 
> Per my response on previous version, it's both, kind of. This is a
> self-emissive panel on

I see, I think I again totally missed that you have there references to
backlight, so binding is in general fine. Describe the hardware in
description field (see commit for mentioned Samsung panel).


Best regards,
Krzysztof