Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754118AbbBJTg6 (ORCPT ); Tue, 10 Feb 2015 14:36:58 -0500 Received: from comal.ext.ti.com ([198.47.26.152]:37459 "EHLO comal.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753282AbbBJTg4 (ORCPT ); Tue, 10 Feb 2015 14:36:56 -0500 Date: Tue, 10 Feb 2015 13:36:21 -0600 From: Benoit Parrot To: Laurent Pinchart CC: "Lad, Prabhakar" , LMML , "devicetree@vger.kernel.org" , LKML , Mauro Carvalho Chehab , Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Grant Likely , Sakari Ailus , Hans Verkuil Subject: Re: [PATCH] media: i2c: add support for omnivision's ov2659 sensor Message-ID: <20150210193620.GA4937@ti.com> References: <1421365163-29394-1-git-send-email-prabhakar.csengg@gmail.com> <3110055.vYzQi51GtK@avalon> <3367184.gDmyGXufvA@avalon> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <3367184.gDmyGXufvA@avalon> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5146 Lines: 126 Laurent Pinchart wrote on Thu [2015-Feb-05 16:52:22 +0200]: > Hi Prabhakar, > > (CC'ing Benoit Parrot) > > On Thursday 05 February 2015 11:55:28 Lad, Prabhakar wrote: > > On Thu, Feb 5, 2015 at 11:53 AM, Laurent Pinchart wrote: > > > On Wednesday 04 February 2015 20:55:02 Lad, Prabhakar wrote: > > >> On Wed, Feb 4, 2015 at 5:03 PM, Laurent Pinchart wrote: > > >> > On Thursday 15 January 2015 23:39:23 Lad, Prabhakar wrote: > > >> >> From: Benoit Parrot > > >> >> > > >> >> this patch adds support for omnivision's ov2659 > > >> >> sensor. > > >> >> > > >> >> Signed-off-by: Benoit Parrot > > >> >> Signed-off-by: Lad, Prabhakar > > >> >> --- > > >> >> > > >> >> .../devicetree/bindings/media/i2c/ov2659.txt | 33 + > > >> >> .../devicetree/bindings/vendor-prefixes.txt | 1 + > > >> >> MAINTAINERS | 10 + > > >> >> drivers/media/i2c/Kconfig | 11 + > > >> >> drivers/media/i2c/Makefile | 1 + > > >> >> drivers/media/i2c/ov2659.c | 1623 +++++++++++ > > >> >> include/media/ov2659.h | 33 + > > >> >> 7 files changed, 1712 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 > > > > > > [snip] > > > > > >> >> diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c > > >> >> new file mode 100644 > > >> >> index 0000000..ce8ec8d > > >> >> --- /dev/null > > >> >> +++ b/drivers/media/i2c/ov2659.c > > >> >> @@ -0,0 +1,1623 @@ > > > > > > [snip] > > > > > >> >> +static const struct ov2659_framesize ov2659_framesizes[] = { > > >> >> + { /* QVGA */ > > >> >> + .width = 320, > > >> >> + .height = 240, > > >> >> + .regs = ov2659_qvga, > > >> >> + .max_exp_lines = 248, > > >> >> + }, { /* VGA */ > > >> >> + .width = 640, > > >> >> + .height = 480, > > >> >> + .regs = ov2659_vga, > > >> >> + .max_exp_lines = 498, > > >> >> + }, { /* SVGA */ > > >> >> + .width = 800, > > >> >> + .height = 600, > > >> >> + .regs = ov2659_svga, > > >> >> + .max_exp_lines = 498, > > >> >> + }, { /* XGA */ > > >> >> + .width = 1024, > > >> >> + .height = 768, > > >> >> + .regs = ov2659_xga, > > >> >> + .max_exp_lines = 498, > > >> >> + }, { /* 720P */ > > >> >> + .width = 1280, > > >> >> + .height = 720, > > >> >> + .regs = ov2659_720p, > > >> >> + .max_exp_lines = 498, > > >> >> + }, { /* SXGA */ > > >> >> + .width = 1280, > > >> >> + .height = 1024, > > >> >> + .regs = ov2659_sxga, > > >> >> + .max_exp_lines = 1048, > > >> >> + }, { /* UXGA */ > > >> >> + .width = 1600, > > >> >> + .height = 1200, > > >> >> + .regs = ov2659_uxga, > > >> >> + .max_exp_lines = 498, > > >> >> + }, > > >> >> +}; > > >> > > > >> > That's what bothers me the most about drivers for Omnivision sensors. > > >> > For some reason (I'd bet on lack of proper documentation) they list a > > >> > couple of supported resolutions with corresponding register values, > > >> > instead of computing the register values from the format configured by > > >> > userspace. That's not the way we want to go. Prabhakar, do you have > > >> > enough documentation to fix that ? > > >> > > >> I am afraid I have limited documentation here. > > > > > > How limited ? :-) I assume someone has documentation, given that the patch > > > contains a larger number of #define's with register names. > > > > Yea I don’t have NDA signed with TI/Omnivision :( because I which I > > lack the documentation. > > And a quick online search doesn't show any leaked datasheet. Wikileaks isn't > doing a good job ;-) > > Benoit, do you think this is something that can be fixed ? Laurent, I did spend several days (many moons ago) to try and derive a generic way to set the output resolution dynamically. However even though the data sheet is somewhat useful, the various working example config from the vendor make use of several un-documented registers which make this pretty much unfeasible. I am afraid we are stuck with this method for the time being. Regards, Benoit Parrot > > -- > Regards, > > Laurent Pinchart > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/