| 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
| ||
|
Date: Wed, 11 Mar 2015 20:09:11 +0200
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Sakari Ailus <sakari.ailus@....fi>
Cc: Lad Prabhakar <prabhakar.csengg@...il.com>,
Sakari Ailus <sakari.ailus@...ux.intel.com>,
Rob Herring <robh+dt@...nel.org>,
Pawel Moll <pawel.moll@....com>,
Mark Rutland <mark.rutland@....com>,
Ian Campbell <ijc+devicetree@...lion.org.uk>,
Kumar Gala <galak@...eaurora.org>,
Mauro Carvalho Chehab <mchehab@....samsung.com>,
Hans Verkuil <hans.verkuil@...co.com>,
LMML <linux-media@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>, devicetree@...r.kernel.org
Subject: Re: [PATCH v4] media: i2c: add support for omnivision's ov2659 sensor
Hi Sakari,
On Wednesday 11 March 2015 13:04:43 Sakari Ailus wrote:
> On Sun, Mar 08, 2015 at 11:33:27AM +0000, Lad Prabhakar wrote:
> > From: Benoit Parrot <bparrot@...com>
> >
> > this patch adds support for omnivision's ov2659
> > sensor, the driver supports following features:
> > 1: Asynchronous probing
> > 2: DT support
> > 3: Media controller support
> >
> > Signed-off-by: Benoit Parrot <bparrot@...com>
> > Signed-off-by: Lad, Prabhakar <prabhakar.csengg@...il.com>
> > ---
> >
> > Sorry quick new version, I need to get this merged for next
> > merge window.
> >
> > Changes for v4:
> > 1: Renamed target frequency property to 'link-frequencies'
> > as per Sakari's suggestion.
> >
> > 2: Changed the copyright to "GPL v2"
> >
> > v3: https://patchwork.kernel.org/patch/5959401/
> > v2: https://patchwork.kernel.org/patch/5859801/
> > v1: https://patchwork.linuxtv.org/patch/27919/
> >
> > .../devicetree/bindings/media/i2c/ov2659.txt | 38 +
> > MAINTAINERS | 10 +
> > drivers/media/i2c/Kconfig | 11 +
> > drivers/media/i2c/Makefile | 1 +
> > drivers/media/i2c/ov2659.c | 1439 +++++++++++++++
> > include/media/ov2659.h | 33 +
> > 6 files changed, 1532 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2659.txt
> > create mode 100644 drivers/media/i2c/ov2659.c
> > create mode 100644 include/media/ov2659.h
> >
> > diff --git a/Documentation/devicetree/bindings/media/i2c/ov2659.txt
> > b/Documentation/devicetree/bindings/media/i2c/ov2659.txt new file mode
> > 100644
> > index 0000000..a655500
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/i2c/ov2659.txt
> > @@ -0,0 +1,38 @@
> > +* 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.
> > +
> > +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.
> > +
> > +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";
> > +
> > + port {
> > + ov2659_0: endpoint {
> > + remote-endpoint = <&vpfe_ep>;
> > + link-frequencies = <70000000>;
>
> link-frequencies = /bits/ 64 <70000000>;
>
> > + };
> > + };
> > + };
> > + ...
> > + };
[snip]
> > new file mode 100644
> > index 0000000..487cb19
> > --- /dev/null
> > +++ b/include/media/ov2659.h
> > @@ -0,0 +1,33 @@
> > +/*
> > + * Omnivision OV2659 CMOS Image Sensor driver
> > + *
> > + * Copyright (C) 2015 Texas Instruments, Inc.
> > + *
> > + * Benoit Parrot <bparrot@...com>
> > + * Lad, Prabhakar <prabhakar.csengg@...il.com>
> > + *
> > + * This program is free software; you may redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; version 2 of the License.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> > + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> > + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> > + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> > + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> > + * SOFTWARE.
> > + */
> > +
> > +#ifndef OV2659_H
> > +#define OV2659_H
> > +
> > +/**
> > + * struct ov2659_platform_data - ov2659 driver platform data
> > + * @link_frequency: target pixel clock frequency
> > + */
> > +struct ov2659_platform_data {
> > + unsigned int link_frequency;
>
> Shouldn't you have xvclk_frequency here as well? In most cases you need to
> set explicitly to a certain value in order to achieve the required link
> frequency.
I'm not sure about that. In the DT case setting the xvclk clock frequency is
done outside of the driver (through assigned-clock-rates for instance).
Wouldn't it make sense to apply the same logic for non-DT platforms and let
the platform configure the clock ?
> > +};
> > +#endif /* OV2659_H */
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists