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

[Alexander Stein <alexander.stein@ew.tq-group.com>]

Hi,

I noticed you missed at least DT bindings maintainer in Cc.
Please use scripts/get_maintainer.pl to get the To/Cc list.

Am Donnerstag, 27. Juni 2024, 19:33:37 CEST schrieb Spencer Hill:
> 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@d3engineering.com>
> > > ---
> > >  .../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@d3engineering.com>
> > > +
> > > +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?

Bindings are independent from implementations, DT bindings maintainer will
tell you ;-)

Take a look at Documentation/devicetree/bindings/media/i2c/sony,imx415.yaml
to get an idea.

> > > +
> > > +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.

Again, driver implementation doesn't matter here. If the hardware (!)
supports both modes, then list it here accordingly.

Thanks and best regards,
Alexander

> > > +
> > > +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@d3engineering.com>
> > > +L:     linux-media@vger.kernel.org
> > > +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@gmail.com>
> > >  M:     Alex Dubov <oakad@yahoo.com>
> > > --
> > > 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.
> 
> 


-- 
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/