linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: u.kleine-koenig@pengutronix.de (Uwe Kleine-König)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCHv5] mx2_camera: Add soc_camera support for i.MX25/i.MX27
Date: Thu, 1 Jul 2010 16:46:23 +0200	[thread overview]
Message-ID: <20100701144622.GG28535@pengutronix.de> (raw)
In-Reply-To: <Pine.LNX.4.64.1007011612580.17489@axis700.grange>

Hi Guennadi,

On Thu, Jul 01, 2010 at 04:23:23PM +0200, Guennadi Liakhovetski wrote:
> > > +config VIDEO_MX2
> > > +	tristate "i.MX27/i.MX25 Camera Sensor Interface driver"
> > > +	depends on VIDEO_DEV && SOC_CAMERA && (MACH_MX27 || ARCH_MX25)
> > > +	select VIDEOBUF_DMA_CONTIG
> > CONTIG?
> 
> What exactly was your question here?
I thought it to be a mistyped "CONFIG", I was wrong.
 
> > > diff --git a/drivers/media/video/mx2_camera.c b/drivers/media/video/mx2_camera.c
> > > new file mode 100644
> > > index 0000000..98c93fa
> > > --- /dev/null
> > > +++ b/drivers/media/video/mx2_camera.c
> > > @@ -0,0 +1,1513 @@
> > 
> > [...snip...]
> 
> > > +static struct platform_driver mx2_camera_driver = {
> > > +	.driver 	= {
> > > +		.name	= MX2_CAM_DRV_NAME,
> > I'm always unsure if you need
> > 
> > 		.owner  = THIS_MODULE,
> > 
> > here.
> 
> It is not needed in this case. See the "owner" field in struct 
> soc_camera_host_ops mx2_soc_camera_host_ops.
> 
> But that's not the reason why I'm replying. What I didn't like in these 
> your reviews, was the fact, that this driver has been submitted a number 
> of times to the arm-kernel ML, it has "mx2" in its subject, so, you had 
> enough chances to review it, just like Sascha did. Instead, you review it 
> now, making the author create new versions of his patch. You have been 
> asked for advise, because this patch potentially collided with other your 
> patches, your help in resolving this question is appreciated. But then you 
> suddenly decide to review the whole patch... Several of my patches have 
> been treated similarly in the past, so, I know how annoying it is to have 
> to re-iterate them because at v5 someone suddenly decided to take part in 
> the patch review process too...
OK it might annoying but still I cannot understand why this is a reason to
lament.  I don't necessarily feel part of the intended audience for each
patch on LAKML that contains mx2 in it's subject, still less if the
patch changes very little in arch/arm/{plat-mxc,arch-mx*} as Baruch's
patch does.  Now he cc:d me and I had a look and even took the time to
point out the things that I noticed.  From my POV a late reviewer is
better than getting code in that is less optimal.  And let me point out
that reviewing sequentially is more efficient in the sum---at least for
the reviewers.

So should I ignore patches that are say already > v3?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

      reply	other threads:[~2010-07-01 14:46 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-07-01 11:03 [PATCHv5] mx2_camera: Add soc_camera support for i.MX25/i.MX27 Baruch Siach
2010-07-01 12:28 ` Uwe Kleine-König
2010-07-01 14:01   ` Baruch Siach
2010-07-01 14:23   ` Guennadi Liakhovetski
2010-07-01 14:46     ` Uwe Kleine-König [this message]

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=20100701144622.GG28535@pengutronix.de \
    --to=u.kleine-koenig@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).