Re: [PATCH v1 1/5] DT bindings for imx492

Laurent Pinchart
2022-11-28T04:38:22Z

Hi Petko,

Thank you for the patch.

The subject line should start with the proper prefix. Look at the git
log in the directory to see what is customary. Furthermore,
s/imx492/Sony IMX492/.

On Fri, Nov 25, 2022 at 05:31:16PM +0200, Petko Manolov wrote:
> Here goes the DT bindings file.

This is also a peculiar commit message :-)

> Signed-off-by: Petko Manolov <petko.manolov@konsulko.com>
> ---
> .../bindings/media/i2c/sony,imx492.yaml | 101 ++++++++++++++++++
> 1 file changed, 101 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/media/i2c/sony,imx492.yaml
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx492.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx492.yaml
> new file mode 100644
> index 000000000000..f4feef17c677
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx492.yaml
> @@ -0,0 +1,101 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/sony,imx492.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Sony imx492 Sensor

s/imx492/IMX492/. Same below. When referring to the hardware, it's
customary to use uppercase.

> +
> +maintainers:
> + - Petko Manolov <petko.manolov@konsulko.com>
> +
> +description:
> +
> + imx492 is a diagonal 23.1 mm (Type 1.4) CMOS image sensor with color or
> + monochrome square pixel array and approximately 47.08 M effective pixels.
> + 12-bit digital output makes it possible to output the signals with high
> + definition for moving pictures. It is programmable through I2C interface. The
> + I2C client address can be either 0x10 or 0x1a depending on SLASEL pin. Image
> + data is sent through MIPI CSI-2.
> +
> +properties:
> + compatible:
> + const: sony,imx492

Missing blank line.

> + reg:
> + description: I2C address

You can drop the description.

> + maxItems: 1
> +
> + assigned-clocks: true
> + assigned-clock-parents: true
> + assigned-clock-rates: true

This shouldn't be needed.

> +
> + clocks:
> + description: Clock frequency 6MHz, 12MHz, 18MHz and 24MHz

If you know the name of the clock pin, I would include it here.

> + maxItems: 1
> +
> + dovdd-supply:
> + description: Interface power supply.

I would list the voltage. Same below.

> +
> + avdd-supply:
> + description: Analog power supply.
> +
> + dvdd-supply:
> + description: Digital power supply.
> +
> + reset-gpios:
> + description: Reference to the GPIO connected to the XCLR pin, if any.
> + maxItems: 1
> +
> + port:
> + additionalProperties: false
> + $ref: /schemas/graph.yaml#/$defs/port-base
> +
> + properties:
> + endpoint:
> + $ref: /schemas/media/video-interfaces.yaml#
> + unevaluatedProperties: false
> +
> + properties:
> + data-lanes: true
> + link-frequencies: true
> +
> + required:
> + - data-lanes
> + - link-frequencies
> +
> + required:
> + - endpoint
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - port
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + i2c0 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + camera@10 {
> + compatible = "sony,imx492";
> + reg = <0x10>;
> + clocks = <&imx492_clk>;
> +
> + assigned-clocks = <&imx492_clk>;
> + assigned-clock-parents = <&imx492_clk_parent>;
> + assigned-clock-rates = <24000000>;

You can drop this from the example, it doesn't showcase the DT bindings
for the IMX492.

> +
> + port {
> + imx492: endpoint {

s/imx492/imx492_out/

> + remote-endpoint = <&cam>;

s/cam/cam_in/

> + data-lanes = <1 2 3 4>;
> + link-frequencies = /bits/ 64 <600000000>;
> + };
> + };
> + };
> + };
> +...

--
Regards,

Laurent Pinchart