Re: [PATCH v1 1/2] dtbindings: media: i2c: Add IMX708 CMOS sensor binding

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 24/01/2023 16:05, Naushir Patuck wrote:
> From: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC.  It might happen, that command when run on an older
kernel, gives you outdated entries.  Therefore please be sure you base
your patches on recent Linux kernel.

> 
> Add YAML devicetree binding for IMX708 CMOS image sensor.
> Let's also add a MAINTAINERS entry for the binding and driver.
> 

1. Use subject prefixes matching the subsystem (which you can get for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching).

2/ Subject: drop second/last, redundant "binding". The "dt-bindings"
prefix is already stating that these are bindings.

> Signed-off-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx>
> Signed-off-by: Naushir Patuck <naush@xxxxxxxxxxxxxxx>
> ---
>  .../devicetree/bindings/media/i2c/imx708.yaml | 119 ++++++++++++++++++
>  MAINTAINERS                                   |   7 ++
>  2 files changed, 126 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/imx708.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/i2c/imx708.yaml b/Documentation/devicetree/bindings/media/i2c/imx708.yaml
> new file mode 100644
> index 000000000000..db1331951fce
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/i2c/imx708.yaml

This must match compatible - missing vendor prefix.

> @@ -0,0 +1,119 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/i2c/imx708.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Sony 1/2.3-Inch 12Mpixel CMOS Digital Image Sensor
> +
> +maintainers:
> +  - Raspberry Pi Kernel Maintenance <kernel-list@xxxxxxxxxxxxxxx>
> +
> +description: |-
> +  The Sony IMX708 is a 1/2.3-inch CMOS active pixel digital image sensor
> +  with an active array size of 4608H x 2592V. It is programmable through
> +  I2C interface. The I2C address is fixed to 0x1A as per sensor data sheet.
> +  Image data is sent through MIPI CSI-2, which is configured as either 2 or
> +  4 data lanes.
> +
> +properties:
> +  compatible:
> +    const: sony,imx708
> +
> +  reg:
> +    description: I2C device address

Drop description.

> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  VDIG-supply:

lowercase, also in other places

> +    description:
> +      Digital I/O voltage supply, 1.1 volts
> +
> +  VANA1-supply:
> +    description:
> +      Analog1 voltage supply, 2.8 volts
> +
> +  VANA2-supply:
> +    description:
> +      Analog2 voltage supply, 1.8 volts
> +
> +  VDDL-supply:
> +    description:
> +      Digital core voltage supply, 1.8 volts
> +
> +  reset-gpios:

maxItems: 1

> +    description: |-
> +      Reference to the GPIO connected to the xclr pin, if any.
> +      Must be released (set high) after all supplies and INCK are applied.
> +
> +  # See ../video-interfaces.txt for more details

I don't think we have this file.

> +  port:
> +    type: object
> +    properties:
> +      endpoint:
> +        type: object
> +        properties:

Take a look at new bindings and use them as example. This is all done
differently.

> +          data-lanes:
> +            description: |-
> +              The sensor supports either two-lane, or four-lane operation.
> +              For two-lane operation the property must be set to <1 2>.
> +            items:
> +              - const: 1
> +              - const: 2
> +
> +          clock-noncontinuous:
> +            type: boolean
> +            description: |-
> +              MIPI CSI-2 clock is non-continuous if this property is present,
> +              otherwise it's continuous.
> +
> +          link-frequencies:
> +            allOf:
> +              - $ref: /schemas/types.yaml#/definitions/uint64-array
> +            description:
> +              Allowed data bus frequencies.
> +
> +        required:
> +          - link-frequencies
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - VANA1-supply
> +  - VANA2-supply
> +  - VDIG-supply
> +  - VDDL-supply
> +  - port
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c0 {

Drop 0

> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        imx708: sensor@1a {
> +            compatible = "sony,imx708";
> +            reg = <0x1a>;
> +            clocks = <&imx708_clk>;
> +            VANA1-supply = <&imx708_vana1>; /* 1.8v */
> +            VANA2-supply = <&imx708_vana2>; /* 2.8v */
> +            VDIG-supply = <&imx708_vdig>;   /* 1.1v */
> +            VDDL-supply = <&imx708_vddl>;   /* 1.8v */
> +
> +            port {
> +                imx708_0: endpoint {
> +                    remote-endpoint = <&csi1_ep>;
> +                    data-lanes = <1 2>;
> +                    clock-noncontinuous;
> +                    link-frequencies = /bits/ 64 <450000000>;
> +                };
> +            };
> +        };
> +    };
> +
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f1c9aa89f6a4..7edeed53de4e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19503,6 +19503,13 @@ T:	git git://linuxtv.org/media_tree.git
>  F:	Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml
>  F:	drivers/media/i2c/imx412.c
>  
> +M:	Raspberry Pi Kernel Maintenance <kernel-list@xxxxxxxxxxxxxxx>
> +L:	linux-media@xxxxxxxxxxxxxxx
> +S:	Maintained
> +T:	git git://linuxtv.org/media_tree.git
> +F:	Documentation/devicetree/bindings/media/i2c/imx708.yaml
> +F:	drivers/media/i2c/imx708.c

There is no such file.


Best regards,
Krzysztof




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux