[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Y9Ef0/lxLS564JT5@kekkonen.localdomain>
Date: Wed, 25 Jan 2023 14:25:55 +0200
From: Sakari Ailus <sakari.ailus@...ux.intel.com>
To: Marco Felsch <m.felsch@...gutronix.de>
Cc: Michael Riesch <michael.riesch@...fvision.net>,
linux-media@...r.kernel.org, devicetree@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
Gerald Loacker <gerald.loacker@...fvision.net>,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Shawn Guo <shawnguo@...nel.org>,
Sascha Hauer <s.hauer@...gutronix.de>,
Pengutronix Kernel Team <kernel@...gutronix.de>,
Fabio Estevam <festevam@...il.com>,
NXP Linux Team <linux-imx@....com>,
Laurent Pinchart <laurent.pinchart+renesas@...asonboard.com>,
Hans Verkuil <hverkuil@...all.nl>,
Mikhail Rudenko <mike.rudenko@...il.com>,
Krzysztof HaĆasa <khalasa@...p.pl>,
Marek Vasut <marex@...x.de>
Subject: Re: [PATCH v2 1/2] dt-bindings: media: i2c: add imx415 cmos image
sensor
Hi Marco,
On Wed, Jan 25, 2023 at 12:21:11PM +0100, Marco Felsch wrote:
> Hi Michael,
>
> On 23-01-24, Michael Riesch wrote:
> > Add devicetree binding for the Sony IMX415 CMOS image sensor.
> >
> > Signed-off-by: Michael Riesch <michael.riesch@...fvision.net>
> > ---
> > v2:
> > - fix reference in port (must be /$defs/port-base)
> > - describe data-lanes in more detail
> > - remove unexpected property clock-lanes from example
> > - sort properties in example alphabetically
> >
> > .../bindings/media/i2c/sony,imx415.yaml | 130 ++++++++++++++++++
> > MAINTAINERS | 7 +
> > 2 files changed, 137 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/media/i2c/sony,imx415.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx415.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx415.yaml
> > new file mode 100644
> > index 000000000000..104c36b64c09
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx415.yaml
> > @@ -0,0 +1,130 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/i2c/sony,imx415.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Sony IMX415 CMOS Image Sensor
> > +
> > +maintainers:
> > + - Michael Riesch <michael.riesch@...fvision.net>
> > +
> > +description: |-
> > + The Sony IMX415 is a diagonal 6.4 mm (Type 1/2.8) CMOS active pixel type
> > + solid-state image sensor with a square pixel array and 8.46 M effective
> > + pixels. This chip operates with analog 2.9 V, digital 1.1 V, and interface
> > + 1.8 V triple power supply, and has low power consumption.
> > + The IMX415 is programmable through I2C interface. The sensor output is
> > + available via CSI-2 serial data output (two or four lanes).
> > +
> > +allOf:
> > + - $ref: ../video-interface-devices.yaml#
> > +
> > +properties:
> > + compatible:
> > + const: sony,imx415
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + clocks:
> > + maxItems: 1
> > +
> > + clock-names:
> > + description: Input clock (24 MHz, 27 MHz, 37.125 MHz, 72 MHz or 74.25 MHz)
>
> I would move this description to the clocks property.
>
> > + items:
> > + - const: inck
>
> If there is only one clock we could remove the clock-names property to
> not bother the user to specify this.
>
> > +
> > + avdd-supply:
> > + description: Analog power supply (2.9 V)
> > +
> > + dvdd-supply:
> > + description: Digital power supply (1.1 V)
> > +
> > + ovdd-supply:
> > + description: Interface power supply (1.8 V)
> > +
> > + reset-gpios:
> > + description: Sensor reset (XCLR) GPIO
> > + maxItems: 1
> > +
> > + flash-leds: true
> > +
> > + lens-focus: true
> > +
> > + orientation: true
> > +
> > + rotation: true
> > +
> > + port:
> > + $ref: /schemas/graph.yaml#/$defs/port-base
> > +
> > + properties:
> > + endpoint:
> > + $ref: /schemas/media/video-interfaces.yaml#
> > + unevaluatedProperties: false
> > +
> > + properties:
> > + data-lanes:
> > + oneOf:
> > + - items:
> > + - const: 1
> > + - const: 2
> > + - items:
> > + - const: 1
> > + - const: 2
> > + - const: 3
> > + - const: 4
> > +
> > + link-frequencies: true
> > +
> > + required:
> > + - data-lanes
> > + - link-frequencies
> > +
> > + required:
> > + - endpoint
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - clocks
> > + - clock-names
> > + - avdd-supply
> > + - dvdd-supply
> > + - ovdd-supply
> > + - port
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/gpio/gpio.h>
> > +
> > + i2c {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + imx415: camera-sensor@1a {
> > + compatible = "sony,imx415";
> > + reg = <0x1a>;
> > + avdd-supply = <&vcc2v9_cam>;
> > + clocks = <&clock_cam>;
> > + clock-names = "inck";
> > + dvdd-supply = <&vcc1v1_cam>;
> > + lens-focus = <&vcm>;
> > + orientation = <2>;
> > + ovdd-supply = <&vcc1v8_cam>;
> > + reset-gpios = <&gpio_expander 14 GPIO_ACTIVE_LOW>;
> > + rotation = <180>;
> > +
> > + port {
> > + imx415_ep: endpoint {
> > + data-lanes = <1 2 3 4>;
> > + link-frequencies = /bits/ 64 <445500000>;
> > + remote-endpoint = <&mipi_in>;
> > + };
> > + };
> > + };
> > + };
>
> Nit albeit I'm not quite sure but I think the indent should be two spaces.
I'm not sure either but AFAIR all the other examples I've seen use four
spaces.
>
> > +...
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index f61eb221415b..c9fa893bf649 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -19493,6 +19493,13 @@ T: git git://linuxtv.org/media_tree.git
> > F: Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml
> > F: drivers/media/i2c/imx412.c
> >
> > +SONY IMX415 SENSOR DRIVER
> > +M: Michael Riesch <michael.riesch@...fvision.net>
> > +L: linux-media@...r.kernel.org
> > +S: Maintained
> > +T: git git://linuxtv.org/media_tree.git
> > +F: Documentation/devicetree/bindings/media/i2c/sony,imx415.yaml
>
> I'm not sure if you should add an entry here since you're already listed
> as maintainer within the binding file.
Should be here as well.
--
Regards,
Sakari Ailus
Powered by blists - more mailing lists