[v3,0/7] media: bindings: Fix handling of video-interface-device

v2->v3:
- s/bindings/dt-bindings/ in $SUBJECT

v1->v2:
- Fix a typo in the ov5640 bindings

As discussed in
https://patchwork.linuxtv.org/project/linux-media/patch/20230928121424.388019-1-festevam@gmail.com/

all properties specified in video-interface-device.yaml are valid for
image sensors.

Some schema however either allow only some of them one by one, or restrict
the supported values for no specific reason.

Fix this by allowing all properties from video-interface-device.yaml
and removing restrictions on the accepted values.

Jacopo Mondi (7):
  media: dt-bindings: hynix,hi846: Add video-interface-device properties
  media: dt-bindings: hynix,hi846: Restrict endpoint properties
  media: dt-bindings: ovti,ov02a10: Fix handling of
    video-interface-device
  media: dt-bindings: ovti,ov4689: Fix handling of
    video-interface-device
  media: dt-bindings: ovti,ov5640: Fix handling of
    video-interface-device
  media: dt-bindings: sony,imx214: Fix handling of
    video-interface-device
  media: dt-bindings: sony,imx415: Fix handling of
    video-interface-device

 .../devicetree/bindings/media/i2c/hynix,hi846.yaml     | 10 ++++++++--
 .../devicetree/bindings/media/i2c/ovti,ov02a10.yaml    |  8 +-------
 .../devicetree/bindings/media/i2c/ovti,ov4689.yaml     |  6 +-----
 .../devicetree/bindings/media/i2c/ovti,ov5640.yaml     |  7 +------
 .../devicetree/bindings/media/i2c/sony,imx214.yaml     |  2 +-
 .../devicetree/bindings/media/i2c/sony,imx415.yaml     | 10 +---------
 6 files changed, 13 insertions(+), 30 deletions(-)

--
2.42.0

On Sat, Sep 30, 2023 at 04:59:46PM +0200, Jacopo Mondi wrote:
> Only properties explicitly listed in the schema are accepted as
> endpoint properties.
> 
> Make sure this is actually enforced by setting 'additionalProperties'
> to false and explicitly allow 'remote-endpoint' in the list of
> endpoint properties.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
>  Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml b/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml
> index 60f19e1152b3..f2ca86501d3c 100644
> --- a/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml
> @@ -58,7 +58,7 @@ properties:
>      properties:
>        endpoint:
>          $ref: /schemas/media/video-interfaces.yaml#
> -        unevaluatedProperties: false
> +        additionalProperties: false

If anything 'additionalProperties' is the exception and 
'unevaluatedProperties' is the rule. Just grep refs to 
video-interfaces.yaml. Why did you change this for just 
this 1 device? 

I'm more worried about undocumented properties than a documented 
property which makes no sense for the h/w being present. So I think 
as-is was fine.

>  
>          properties:
>            data-lanes:
> @@ -73,6 +73,7 @@ properties:
>                    - const: 2
>  
>            link-frequencies: true

I suppose we could remove this as it has no effect on the schema 
validation, but it's probably worthwhile to keep for documentation 
purposes.

Rob

> +          remote-endpoint: true
>  
>          required:
>            - data-lanes
> -- 
> 2.42.0
>

On Sat, 30 Sep 2023 16:59:47 +0200, Jacopo Mondi wrote:
> Fix handling of properties from video-interface-device.yaml for
> Omnivision OV02A10 sensor.
> 
> There is no reason to restrict the allowed rotation degrees to 0 and 180,
> as the sensor can be mounted with any rotation.
> 
> Also, as all the properties described by video-interface-device.yaml are
> allowed for the image sensor, make them accepted by changing
> "additionalProperties: false" to "unevaluatedProperties: false" at the
> schema top-level.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> ---
>  .../devicetree/bindings/media/i2c/ovti,ov02a10.yaml       | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Hi Jacopo,

On Sat, Sep 30, 2023 at 04:59:45PM +0200, Jacopo Mondi wrote:
> Allow properties from video-interface-device.yaml for the SK Hynix Hi-846
> sensor.
> 
> All properties specified in video-interface-device.yaml schema are
> valid, so make them accepted by changing "additionalProperties: false"
> to "unevaluatedProperties: false" at the schema top-level.

The patch seems fine to me, but I wonder if we should change the title of
video-interface-devices.yaml (it's plural) to something that refers to
camera sensors, and possibly split it. It's currently not relevant for
other types of devices.

On Tue, Oct 03, 2023 at 08:27:30AM +0000, Sakari Ailus wrote:
> Hi Jacopo,
> 
> On Sat, Sep 30, 2023 at 04:59:45PM +0200, Jacopo Mondi wrote:
> > Allow properties from video-interface-device.yaml for the SK Hynix Hi-846
> > sensor.
> > 
> > All properties specified in video-interface-device.yaml schema are
> > valid, so make them accepted by changing "additionalProperties: false"
> > to "unevaluatedProperties: false" at the schema top-level.
> 
> The patch seems fine to me, but I wonder if we should change the title of
> video-interface-devices.yaml (it's plural) to something that refers to
> camera sensors, and possibly split it. It's currently not relevant for
> other types of devices.

I was thinking exactly the same yesterday.