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 ?
Thanks
j
> .../devicetree/bindings/media/i2c/hynix,hi846.yaml | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml b/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml
> index 1e2df8cf2937..8b7a46a15458 100644
> --- a/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml
> @@ -14,6 +14,9 @@ description: |-
> interface and CCI (I2C compatible) control bus. The output format
> is raw Bayer.
>
> +allOf:
> + - $ref: /schemas/media/video-interface-devices.yaml#
> +
> properties:
> compatible:
> const: hynix,hi846
> @@ -48,6 +51,10 @@ properties:
> vddd-supply:
> description: Definition of the regulator used for the VDDD power supply.
>
> + orientation: true
> +
> + rotation: true
> +
> port:
> $ref: /schemas/graph.yaml#/$defs/port-base
> unevaluatedProperties: false
> --
> 2.34.1
>
