Re: [PATCH v6 1/4] dt-bindings: media: imx258: add bindings for IMX258 sensor

From: Sakari Ailus
Date: Fri Jan 22 2021 - 06:48:57 EST


Hi Krysztof,

On Wed, Nov 18, 2020 at 09:27:12PM +0100, Krzysztof Kozlowski wrote:
> Add bindings for the IMX258 camera sensor. The bindings, just like the
> driver, are quite limited, e.g. do not support regulator supplies.
>
> Signed-off-by: Krzysztof Kozlowski <krzk@xxxxxxxxxx>
> Reviewed-by: Rob Herring <robh@xxxxxxxxxx>
>
> ---
>
> Changes since v4:
> 1. Add clock-lanes,
> 2. Add Rob's review,
> 3. Add one more example and extend existing one,
> 4. Add common clock properties (assigned-*).
>
> Changes since v3:
> 1. Document also two lane setup.
>
> Changes since v2:
> 1. Remove clock-frequency, add reset GPIOs, add supplies.
> 2. Use additionalProperties.
>
> Changes since v1:
> 1. None
> ---
> .../devicetree/bindings/media/i2c/imx258.yaml | 140 ++++++++++++++++++
> MAINTAINERS | 1 +
> 2 files changed, 141 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/i2c/imx258.yaml
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/imx258.yaml b/Documentation/devicetree/bindings/media/i2c/imx258.yaml
> new file mode 100644
> index 000000000000..4a3471fb88a1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/imx258.yaml
> @@ -0,0 +1,140 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/imx258.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Sony IMX258 13 Mpixel CMOS Digital Image Sensor
> +
> +maintainers:
> + - Krzysztof Kozlowski <krzk@xxxxxxxxxx>
> +
> +description: |-
> + IMX258 is a diagonal 5.867mm (Type 1/3.06) 13 Mega-pixel CMOS active pixel
> + type stacked image sensor with a square pixel array of size 4208 x 3120. It
> + is programmable through I2C interface. Image data is sent through MIPI
> + CSI-2.
> +
> +properties:
> + compatible:
> + const: sony,imx258
> +
> + assigned-clocks: true
> + assigned-clock-parents: true
> + assigned-clock-rates: true
> +
> + clocks:
> + description:
> + Clock frequency from 6 to 27 MHz.
> + maxItems: 1
> +
> + reg:
> + maxItems: 1
> +
> + reset-gpios:
> + description: |-
> + Reference to the GPIO connected to the XCLR pin, if any.
> +
> + vana-supply:
> + description:
> + Analog voltage (VANA) supply, 2.7 V
> +
> + vdig-supply:
> + description:
> + Digital I/O voltage (VDIG) supply, 1.2 V
> +
> + vif-supply:
> + description:
> + Interface voltage (VIF) supply, 1.8 V
> +
> + # See ../video-interfaces.txt for more details
> + port:
> + type: object
> + properties:
> + endpoint:
> + type: object
> + properties:
> + clock-lanes:
> + const: 0

This is redundant. Please remove, same for the examples. Can be a separate
patch, too.

With this change the set seems good to me.

--
Sakari Ailus