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

[Spencer Hill <shill@d3engineering.com>]

On Thu, Jun 27, 2024 at 08:52:47AM +0200, Krzysztof Kozlowski wrote:
> On 26/06/2024 23:15, Spencer Hill wrote:
> > Add bindings for Sony IMX728.
> >
> > Signed-off-by: Spencer Hill <shill@d3engineering.com>
>
> Since this was not tested, limited review follows:
>
> > ---
> >  .../bindings/media/i2c/sony,imx728.yaml       | 78 +++++++++++++++++++
>
> Bindings are before users.
>

Sorry I am not sure what you are recommending here, should I change the
order of the patches so that the dt-binding patch is first?

> >  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: |-
>
> Do not need '|-' unless you need to preserve formatting.
>

This will be needed once I add more information.

> > +  Sony IMX728 camera sensor.
>
> That's duplicating title. Say something more about hardware or just drop.
>

I will add some more information about the sensor here.

> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - sony,imx728
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  clock-names:
> > +    const: inck
> > +
> > +  xclr-gpios:
> > +    maxItems: 1
> > +    description:
> > +      Specifier for the GPIO connected to the XCLR (System Reset) pin.
> > +
> > +  port:
> > +    $ref: /schemas/graph.yaml#/properties/port
> > +    additionalProperties: false
> > +
> > +    properties:
> > +      endpoint:
> > +        $ref: ../video-interfaces.yaml#
> > +        unevaluatedProperties: false
> > +
> > +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
>
> Drop this T:. You do not maintain that git tree so it is already covered
> by subsystem entry.
>

Okay

> > +F:     Documentation/devicetree/bindings/media/i2c/sony,imx728.yaml
> > +F:     drivers/media/i2c/imx728.c
>
> .[ch] or *
>

The .h file will be removed in an updated patch set in response to other
feedback.

>
>
> Best regards,
> Krzysztof
>

I'll make these changes

Thanks,
Spencer
Please be aware that this email includes email addresses outside of the organization.