All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marek Vasut <marek.vasut@gmail.com>
To: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Cc: Linux Media Mailing List <linux-media@vger.kernel.org>
Subject: Re: V4L2: Add a v4l2-subdev (soc-camera) driver for OmniVision OV9640 sensor
Date: Tue, 15 Sep 2009 00:06:00 +0200	[thread overview]
Message-ID: <200909150006.00150.marek.vasut@gmail.com> (raw)
In-Reply-To: <Pine.LNX.4.64.0909142319240.4359@axis700.grange>

Dne Po 14. září 2009 23:21:33 Guennadi Liakhovetski napsal(a):
> On Mon, 14 Sep 2009, Marek Vasut wrote:
> > Dne Po 14. září 2009 22:30:41 Guennadi Liakhovetski napsal(a):
> > > On Mon, 14 Sep 2009, Marek Vasut wrote:
> > > > Dne Po 14. září 2009 21:29:26 Guennadi Liakhovetski napsal(a):
> > > > > From: Marek Vasut <marek.vasut@gmail.com>
> > > > >
> > > > > Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> > > > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > > > > ---
> > > > >
> > > > > Marek, please confirm, that this version is ok. I'll push it
> > > > > upstream for 2.6.32 then.
> > > >
> > > > No, it's not OK. You removed the RGB part. Either enclose those parts
> > > > into ifdef OV9640_RGB_BUGGY or preserve it in some other way. Someone
> > > > will certainly want to re-add RGB parts later and will have to figure
> > > > it out all over again.
> > >
> > > Ok, make a proposal, how you would like to see it. But - I do not want
> > > commented out code, including "#ifdef MACRO_THAT_DOESNT_GET_DEFINED." I
> > > think, I described it in sufficient detail, so that re-adding that code
> > > should not take longer than 10 minutes for anyone sufficiently familiar
> > > with the code. Referencing another driver also has an advantage, that
> > > if we switch to imagebus or any other API, you don't get stale
> > > commented out code, but you look up updated code in a functional
> > > driver. But I am open to your ideas / but no commented out code,
> > > please.
> >
> > The RGB is broken only in a way where it swaps colours, the color matrix
> > coeficients and register configurations are OK (which is what other
> > people who will want to add it will need to figure out again from scratch
> > if you remove the code).
> 
> Excuse me, have you looked at my patch? Have you compared it to yours? I
> only removed your RGB code entries from the list of supported formats, I
> haven't removed any implementation details, thus effectively just
> disabling it.

Just briefly skimmed over it. Ok then, that diff seems fine. I assume the imagebus 
will fix the rgb issues anyway.
> 
> > I dont want this merged before this is solved in some way where those
> > values are preserved.
> 
> Sure, let's have it fixed and submit it in time for 2.6.33.
> 
> Thanks
> Guennadi
> ---
> Guennadi Liakhovetski, Ph.D.
> Freelance Open-Source Software Developer
> http://www.open-technology.de/
> 

  reply	other threads:[~2009-09-14 22:06 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-08-22  6:50 [PATCH 2/3] Add driver for OmniVision OV9640 sensor Marek Vasut
2009-08-24 18:06 ` Guennadi Liakhovetski
2009-09-04 20:25   ` Marek Vasut
2009-09-11 21:51     ` Guennadi Liakhovetski
2009-09-13 16:43       ` Marek Vasut
2009-09-13 19:13         ` Guennadi Liakhovetski
2009-09-14 14:35           ` Marek Vasut
2009-09-14 14:39             ` Marek Vasut
2009-09-14 14:45             ` Guennadi Liakhovetski
2009-09-14 16:29               ` Marek Vasut
2009-09-14 19:29                 ` V4L2: Add a v4l2-subdev (soc-camera) " Guennadi Liakhovetski
2009-09-14 20:02                   ` Marek Vasut
2009-09-14 20:30                     ` Guennadi Liakhovetski
2009-09-14 20:57                       ` David Ellingsworth
2009-09-14 21:15                       ` Marek Vasut
2009-09-14 21:21                         ` Guennadi Liakhovetski
2009-09-14 22:06                           ` Marek Vasut [this message]
2009-09-15 19:55                             ` Guennadi Liakhovetski
2009-09-15 21:38                               ` Marek Vasut
2009-09-14 14:26       ` [PATCH 2/3] Add " Marek Vasut
2009-09-14 14:41         ` Guennadi Liakhovetski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200909150006.00150.marek.vasut@gmail.com \
    --to=marek.vasut@gmail.com \
    --cc=g.liakhovetski@gmx.de \
    --cc=linux-media@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.