On Thu, Sep 28, 2023 at 04:57:23PM +0200, Jacopo Mondi wrote:
> Hi Fabio, Krzysztof
>
> On Thu, Sep 28, 2023 at 09:14:24AM -0300, Fabio Estevam wrote:
> > From: Fabio Estevam <festevam@xxxxxxx>
> >
> > Document the 'orientation' and 'rotation' properties, which
> > are valid for the SK Hynix Hi-846 sensor.
> >
> > Their definitions come from video-interface-devices.yaml, so add
> > a reference to it.
> >
> > Signed-off-by: Fabio Estevam <festevam@xxxxxxx>
> > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
> > Acked-by: Martin Kepplinger <martink@xxxxxxxxx>
> > ---
> > Changes since v1:
> > - Also pass a ref to video-interface-devices.yaml. (Martin)
> >
>
> This patch is technically correct, so this message is not meant to
> delay its integration or anything like that, but I'll take the
> occasion to ask for guidance to the DT maintainers, as I think this
> approach doesn't scale.
>
> I count the following sensor bindings that refer to
> video-interface-device.yaml
>
> Documentation/devicetree/bindings/media/i2c/ovti,ov02a10.yaml
> Documentation/devicetree/bindings/media/i2c/ovti,ov4689.yaml
> Documentation/devicetree/bindings/media/i2c/ovti,ov5640.yaml
> Documentation/devicetree/bindings/media/i2c/ovti,ov5675.yaml
> Documentation/devicetree/bindings/media/i2c/ovti,ov5693.yaml
> Documentation/devicetree/bindings/media/i2c/sony,imx214.yaml
> Documentation/devicetree/bindings/media/i2c/sony,imx415.yaml
>
> These:
>
> Documentation/devicetree/bindings/media/i2c/ovti,ov02a10.yaml:additionalProperties: false
> Documentation/devicetree/bindings/media/i2c/ovti,ov4689.yaml:additionalProperties: false
> Documentation/devicetree/bindings/media/i2c/ovti,ov5640.yaml:additionalProperties: false
> Documentation/devicetree/bindings/media/i2c/sony,imx214.yaml:additionalProperties: false
> Documentation/devicetree/bindings/media/i2c/sony,imx415.yaml:additionalProperties: false
>
> specify 'additionalProperties: false' at the top-level.
> This is correct imho, as it implies that any properties not
> specifically allowed by bindings is forbidden.
>
> This unfortunately applies to 'rotation' and 'orientation' as well.
> This is not correct, as those two properties should apply to all
> sensors without the requiring the bindings to explicitly allow them.
>
> Counterproof: It's very easy to break validation of, in example,
> ov5640
>
> --- a/Documentation/devicetree/bindings/media/i2c/ovti,ov5640.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5640.yaml
> @@ -109,6 +109,7 @@ examples:
> powerdown-gpios = <&gpio1 19 GPIO_ACTIVE_HIGH>;
> reset-gpios = <&gpio1 20 GPIO_ACTIVE_LOW>;
> rotation = <180>;
> + orientation = <0>;
>
> port {
> /* MIPI CSI-2 bus endpoint */
>
> $ make ARCH=arm64 dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/media/i2c/ovti,ov5640.yaml
> DTC_CHK Documentation/devicetree/bindings/media/i2c/ovti,ov5640.example.dtb
> 'orientation' does not match any of the regexes: 'pinctrl-[0-9]+'
> from schema $id: http://devicetree.org/schemas/media/i2c/ovti,ov5640.yaml#
>
> Is there a way to allow those two properties ('rotation' and
> 'orientation') to be accepted by all sensor drivers bindings ?
Use unevaluatedProperties instead of additionalProperties and add a $ref
to video-interface-devices.yaml in the sensor schemas. However, that
will allow all properties in video-interface-devices.yaml (which is just
flash-leds and lens-focus which seem fine). If you don't want that, then
you will have to split up video-interface-devices.yaml.
Rob
