linux-kernel - Re: [PATCH 1/1] media: dt-bindings: move ovti,ov2659.txt into ovti,ov8858.yaml

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aPekq38Sr5KvXvKw@lizhi-Precision-Tower-5810>
Date: Tue, 21 Oct 2025 11:20:11 -0400
From: Frank Li <Frank.li@....com>
To: Sakari Ailus <sakari.ailus@....fi>
Cc: "Lad, Prabhakar" <prabhakar.csengg@...il.com>,
	Mauro Carvalho Chehab <mchehab@...nel.org>,
	Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Jacopo Mondi <jacopo.mondi@...asonboard.com>,
	Nicholas Roth <nicholas@...hemail.net>,
	"open list:OV2659 OMNIVISION SENSOR DRIVER" <linux-media@...r.kernel.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" <devicetree@...r.kernel.org>,
	open list <linux-kernel@...r.kernel.org>, imx@...ts.linux.dev
Subject: Re: [PATCH 1/1] media: dt-bindings: move ovti,ov2659.txt into
 ovti,ov8858.yaml

On Tue, Oct 21, 2025 at 11:43:20AM +0300, Sakari Ailus wrote:
> Hi Frank,
>
> Thanks for the patch.
>
> On Fri, Sep 12, 2025 at 02:30:02PM -0400, Frank Li wrote:
> > The properties in ovti,ov2659.txt are the same as ovti,ov8858. So move it
> > to this yaml file.
> >
> > Signed-off-by: Frank Li <Frank.Li@....com>
> > ---
> >  .../bindings/media/i2c/ovti,ov2659.txt        | 47 -------------------
> >  .../bindings/media/i2c/ovti,ov8858.yaml       |  4 +-
> >  2 files changed, 3 insertions(+), 48 deletions(-)
> >  delete mode 100644 Documentation/devicetree/bindings/media/i2c/ovti,ov2659.txt
> >
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov2659.txt b/Documentation/devicetree/bindings/media/i2c/ovti,ov2659.txt
> > deleted file mode 100644
> > index 92989a619f292..0000000000000
> > --- a/Documentation/devicetree/bindings/media/i2c/ovti,ov2659.txt
> > +++ /dev/null
> > @@ -1,47 +0,0 @@
> > -* OV2659 1/5-Inch 2Mp SOC Camera
> > -
> > -The Omnivision OV2659 is a 1/5-inch SOC camera, with an active array size of
> > -1632H x 1212V. It is programmable through a SCCB. The OV2659 sensor supports
> > -multiple resolutions output, such as UXGA, SVGA, 720p. It also can support
> > -YUV422, RGB565/555 or raw RGB output formats.
>
> I don't object merging this with the ov8858 bindings as such but the
> differences between the sensors should be taken into account. Something
> like the text above should be added to those bindings, which now describe
> the ov8858 only. Also, the number of lanes is probably different between
> the two.
>
> What about the regulators? The ov2659 doesn't control any right now, but
> that's probably an oversight (and whatever regulators connected on the
> platform used were hard-wired).
>
> Overall, it might be easier to keep the two bindings separate rather than
> have some number of ifs in the bindings.

The key is what's differences between two bindings. Most ov sensors use
similar properties. Duplicate these information at bunch binding files is
more difficult to maintain it. It is more hard to standardize if use
difference files because seldom check other files when add new one.

For example: clock-names, some use xvclk, some use xclk, but it points to
the same things.

Frank

>
> You can drop clock-names as there's just a single clock (the same goes for
> the driver naturally).
>
> > -
> > -Required Properties:
> > -- compatible: Must be "ovti,ov2659"
> > -- reg: I2C slave address
> > -- clocks: reference to the xvclk input clock.
> > -- clock-names: should be "xvclk".
> > -- link-frequencies: target pixel clock frequency.
> > -
> > -Optional Properties:
> > -- powerdown-gpios: reference to the GPIO connected to the pwdn pin, if any.
> > -  Active high with internal pull down resistor.
> > -- reset-gpios: reference to the GPIO connected to the resetb pin, if any.
> > -  Active low with internal pull up resistor.
> > -
> > -For further reading on port node refer to
> > -Documentation/devicetree/bindings/media/video-interfaces.txt.
> > -
> > -Example:
> > -
> > -	i2c0@...2000 {
> > -		...
> > -		...
> > -		 ov2659@30 {
> > -			compatible = "ovti,ov2659";
> > -			reg = <0x30>;
> > -
> > -			clocks = <&clk_ov2659 0>;
> > -			clock-names = "xvclk";
> > -
> > -			powerdown-gpios = <&gpio6 14 GPIO_ACTIVE_HIGH>;
> > -			reset-gpios = <&gpio6 15 GPIO_ACTIVE_LOW>;
> > -
> > -			port {
> > -				ov2659_0: endpoint {
> > -					remote-endpoint = <&vpfe_ep>;
> > -					link-frequencies = /bits/ 64 <70000000>;
> > -				};
> > -			};
> > -		};
> > -		...
> > -	};
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov8858.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov8858.yaml
> > index 491f2931e6bcd..d7059dbee7fca 100644
> > --- a/Documentation/devicetree/bindings/media/i2c/ovti,ov8858.yaml
> > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov8858.yaml
> > @@ -19,7 +19,9 @@ $ref: /schemas/media/video-interface-devices.yaml#
> >
> >  properties:
> >    compatible:
> > -    const: ovti,ov8858
> > +    enum:
> > +      - ovti,ov2659
> > +      - ovti,ov8858
> >
> >    reg:
> >      maxItems: 1
>
> --
> Kind regards,
>
> Sakari Ailus