Re: [PATCH] media: dt-bindings: i2c: ovti,ov5640: Drop ref to video-interface-devices.yaml

From: Krzysztof Kozlowski
Date: Mon Sep 19 2022 - 04:19:18 EST


On 19/09/2022 10:08, Lad, Prabhakar wrote:
> Hi Laurent,
>
> Thank you for the review.
>
> On Sun, Sep 18, 2022 at 12:06 AM Laurent Pinchart
> <laurent.pinchart@xxxxxxxxxxxxxxxx> wrote:
>>
>> Hi Prabhakar,
>>
>> Thank you for the patch.
>>
>> On Fri, Sep 16, 2022 at 02:35:21PM +0100, Prabhakar wrote:
>>> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
>>>
>>> video-interface-devices.yaml isn't used so just drop it from the
>>> DT binding doc.
>>>
>>> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
>>> ---
>>> Documentation/devicetree/bindings/media/i2c/ovti,ov5640.yaml | 3 ---
>>> 1 file changed, 3 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5640.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5640.yaml
>>> index 540fd69ac39f..ce99aada75ad 100644
>>> --- a/Documentation/devicetree/bindings/media/i2c/ovti,ov5640.yaml
>>> +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5640.yaml
>>> @@ -9,9 +9,6 @@ title: OmniVision OV5640 Image Sensor Device Tree Bindings
>>> maintainers:
>>> - Steve Longerbeam <slongerbeam@xxxxxxxxx>
>>>
>>> -allOf:
>>> - - $ref: /schemas/media/video-interface-devices.yaml#
>>> -
>>
>> The rotation property listed in this binding uses the definition from
>> video-interface-devices.yaml. I don't think just dropping this is the
>> right solution. Changing additionaProperties to unevaluatedProperties
>> seems a better option.
>>
> Agreed, I missed rotation was used from video-interface-devices.yaml.
> Agreed the changing additionaProperties to unevaluatedProperties seems
> a better option.

The meaning of unevaluatedProperties:false would be here - accept other
properties (not mentioned here explicitly) from referenced schema. If
this is your actual intention for this binding, it makes sense. But if
the intention in this binding was to disallow these other properties,
then it would be wrong to change to unevaluatedProperties.

Therefore before sending patches and calling something better or not,
please instead focus on that aspect of referenced schema.


Best regards,
Krzysztof