Re: [PATCH 2/2] media: dt-bindings: Add Sony IMX728

[Spencer Hill <shill@xxxxxxxxxxxxxxxxx>]

On Thu, Jun 27, 2024 at 03:38:03PM +0200, Alexander Stein wrote:
> Hi Spencer,
>
> thanks for the patch.
>
> Am Mittwoch, 26. Juni 2024, 23:15:29 CEST schrieb Spencer Hill:
> > Add bindings for Sony IMX728.
> >
> > Signed-off-by: Spencer Hill <shill@xxxxxxxxxxxxxxxxx>
> > ---
> >  .../bindings/media/i2c/sony,imx728.yaml       | 78 +++++++++++++++++++
> >  MAINTAINERS                                   |  9 +++
> >  2 files changed, 87 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/i2c/sony,imx728.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx728.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx728.yaml
> > new file mode 100644
> > index 000000000000..613042ab5abe
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx728.yaml
> > @@ -0,0 +1,78 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/i2c/sony,imx728.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Sony IMX728 Camera Sensor
> > +
> > +maintainers:
> > +  - Spencer Hill <shill@xxxxxxxxxxxxxxxxx>
> > +
> > +description: |-
> > +  Sony IMX728 camera sensor.
>
> Are there some more information? Like max resolution, image format, bpp, framerate, etc.
>

I will add some more descriptive information about the sensor. Should
information that is only relevant to the driver be included here? Or
should I just describe the sensor? For example, the sensor supports a
significant number of different modes, however only a single one is
implemented in the driver at the moment, should I list the possible
modes, or just the supported ones?

> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - sony,imx728
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  clock-names:
> > +    const: inck
>
> Are there any restrictions about frequency? Like a specific set of
> frequencies?
>

The sensor must be between 18MHz and 30MHz, I will add this to the
description.

> > +
> > +  xclr-gpios:
>
> reset-gpios, see sony.imx290.yaml
>

I will rename this here and in the driver.

> > +    maxItems: 1
> > +    description:
> > +      Specifier for the GPIO connected to the XCLR (System Reset) pin.
>
> No voltage supplies?
>

I will add these.

> > +
> > +  port:
> > +    $ref: /schemas/graph.yaml#/properties/port
> > +    additionalProperties: false
> > +
> > +    properties:
> > +      endpoint:
> > +        $ref: ../video-interfaces.yaml#
> > +        unevaluatedProperties: false
>
> Which data-lane configuration is allowed? 4 lanes only? or 2 lanes?
>
> Best regards,
> Alexander
>

The sensor supports both 4 and 2 lane modes, though only 4 is
implemented at the moment.

> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - clock-names
> > +  - port
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/gpio/gpio.h>
> > +
> > +    i2c {
> > +        clock-frequency = <400000>;
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        camera@1a {
> > +            compatible = "sony,imx728";
> > +            reg = <0x1a>;
> > +
> > +            clocks = <&fixed_clock>;
> > +            clock-names = "inck";
> > +
> > +            xclr-gpios = <&gpio4 17 GPIO_ACTIVE_LOW>;
> > +
> > +            port {
> > +                camera1: endpoint {
> > +                    remote-endpoint = <&vin1a_ep>;
> > +                };
> > +            };
> > +        };
> > +    };
> > +
> > +...
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index ef6be9d95143..34fde35eb0bd 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -20589,6 +20589,15 @@ T:     git git://linuxtv.org/media_tree.git
> >  F:     Documentation/devicetree/bindings/media/i2c/sony,imx415.yaml
> >  F:     drivers/media/i2c/imx415.c
> >
> > +SONY IMX728 SENSOR DRIVER
> > +M:     Spencer Hill <shill@xxxxxxxxxxxxxxxxx>
> > +L:     linux-media@xxxxxxxxxxxxxxx
> > +S:     Maintained
> > +T:     git git://linuxtv.org/media_tree.git
> > +F:     Documentation/devicetree/bindings/media/i2c/sony,imx728.yaml
> > +F:     drivers/media/i2c/imx728.c
> > +F:     drivers/media/i2c/imx728.h
> > +
> >  SONY MEMORYSTICK SUBSYSTEM
> >  M:     Maxim Levitsky <maximlevitsky@xxxxxxxxx>
> >  M:     Alex Dubov <oakad@xxxxxxxxx>
> > --
> > 2.40.1
> >
> > Please be aware that this email includes email addresses outside of the organization.
> >
> >
>
>
> --
> TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> Amtsgericht München, HRB 105018
> Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> http://www.tq-group.com/
>
>
Please be aware that this email includes email addresses outside of the organization.