mbox

[v3,0/9] Media Controller capture driver for DM365

Message ID 1354099329-20722-1-git-send-email-prabhakar.lad@ti.com (mailing list archive)
State RFC, archived
Headers

Pull-request

git://linuxtv.org/mhadli/v4l-dvb-davinci_devices.git vpfe_driver_staging

Message

Prabhakar Nov. 28, 2012, 10:42 a.m. UTC
From: Manjunath Hadli <manjunath.hadli@ti.com>

Mauro/Greg,
 The below series of patches have gone through good amount of reviews, and
agreed by Laurent, Hans and Sakari to be part of the staging tree. I am combining
the patchs with the pull request so we can get them into the 3.8 kernel.
Please pull these patches.If you want a seperate pull request, please let me
know.

This patch set adds media controller based capture driver for
DM365.

This driver bases its design on Laurent Pinchart's Media Controller Design
whose patches for Media Controller and subdev enhancements form the base.
The driver also takes copious elements taken from Laurent Pinchart and
others' OMAP ISP driver based on Media Controller. So thank you all the
people who are responsible for the Media Controller and the OMAP ISP driver.

Also, the core functionality of the driver comes from the arago vpfe capture
driver of which the isif capture was based on V4L2, with other drivers like
ipipe, ipipeif and Resizer.

Changes for v2:
1: Migrated the driver for videobuf2 usage pointed Hans.
2: Changed the design as pointed by Laurent, Exposed one more subdevs
   ipipeif and split the resizer subdev into three subdevs.
3: Rearrganed the patch sequence and changed the commit messages.
4: Changed the file architecture as pointed by Laurent.

Changes for v3:
1: Rebased on staging.
2: Seprated out patches which would go into staging.

The following changes since commit c6c22955f80f2db9614b01fe5a3d1cfcd8b3d848:

  [media] dma-mapping: fix dma_common_get_sgtable() conditional compilation (2012-11-27 09:42:31 -0200)

are available in the git repository at:
  git://linuxtv.org/mhadli/v4l-dvb-davinci_devices.git vpfe_driver_staging

Manjunath Hadli (9):
      davinci: vpfe: add v4l2 capture driver with media interface
      davinci: vpfe: add v4l2 video driver support
      davinci: vpfe: dm365: add IPIPEIF driver based on media framework
      davinci: vpfe: dm365: add ISIF driver based on media framework
      davinci: vpfe: dm365: add IPIPE support for media controller driver
      davinci: vpfe: dm365: add IPIPE hardware layer support
      davinci: vpfe: dm365: resizer driver based on media framework
      davinci: vpfe: dm365: add build infrastructure for capture driver
      davinci: vpfe: Add documentation and TODO

 drivers/staging/media/Kconfig                      |    2 +
 drivers/staging/media/Makefile                     |    1 +
 drivers/staging/media/davinci_vpfe/Kconfig         |    9 +
 drivers/staging/media/davinci_vpfe/Makefile        |    3 +
 drivers/staging/media/davinci_vpfe/TODO            |   34 +
 .../staging/media/davinci_vpfe/davinci-vpfe-mc.txt |  154 ++
 .../staging/media/davinci_vpfe/davinci_vpfe_user.h | 1290 ++++++++++++
 drivers/staging/media/davinci_vpfe/dm365_ipipe.c   | 1863 +++++++++++++++++
 drivers/staging/media/davinci_vpfe/dm365_ipipe.h   |  179 ++
 .../staging/media/davinci_vpfe/dm365_ipipe_hw.c    | 1048 ++++++++++
 .../staging/media/davinci_vpfe/dm365_ipipe_hw.h    |  559 ++++++
 drivers/staging/media/davinci_vpfe/dm365_ipipeif.c | 1071 ++++++++++
 drivers/staging/media/davinci_vpfe/dm365_ipipeif.h |  233 +++
 .../media/davinci_vpfe/dm365_ipipeif_user.h        |   93 +
 drivers/staging/media/davinci_vpfe/dm365_isif.c    | 2104 ++++++++++++++++++++
 drivers/staging/media/davinci_vpfe/dm365_isif.h    |  203 ++
 .../staging/media/davinci_vpfe/dm365_isif_regs.h   |  294 +++
 drivers/staging/media/davinci_vpfe/dm365_resizer.c | 1999 +++++++++++++++++++
 drivers/staging/media/davinci_vpfe/dm365_resizer.h |  244 +++
 drivers/staging/media/davinci_vpfe/vpfe.h          |   86 +
 .../staging/media/davinci_vpfe/vpfe_mc_capture.c   |  740 +++++++
 .../staging/media/davinci_vpfe/vpfe_mc_capture.h   |   97 +
 drivers/staging/media/davinci_vpfe/vpfe_video.c    | 1620 +++++++++++++++
 drivers/staging/media/davinci_vpfe/vpfe_video.h    |  155 ++
 24 files changed, 14081 insertions(+), 0 deletions(-)
 create mode 100644 drivers/staging/media/davinci_vpfe/Kconfig
 create mode 100644 drivers/staging/media/davinci_vpfe/Makefile
 create mode 100644 drivers/staging/media/davinci_vpfe/TODO
 create mode 100644 drivers/staging/media/davinci_vpfe/davinci-vpfe-mc.txt
 create mode 100644 drivers/staging/media/davinci_vpfe/davinci_vpfe_user.h
 create mode 100644 drivers/staging/media/davinci_vpfe/dm365_ipipe.c
 create mode 100644 drivers/staging/media/davinci_vpfe/dm365_ipipe.h
 create mode 100644 drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c
 create mode 100644 drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.h
 create mode 100644 drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
 create mode 100644 drivers/staging/media/davinci_vpfe/dm365_ipipeif.h
 create mode 100644 drivers/staging/media/davinci_vpfe/dm365_ipipeif_user.h
 create mode 100644 drivers/staging/media/davinci_vpfe/dm365_isif.c
 create mode 100644 drivers/staging/media/davinci_vpfe/dm365_isif.h
 create mode 100644 drivers/staging/media/davinci_vpfe/dm365_isif_regs.h
 create mode 100644 drivers/staging/media/davinci_vpfe/dm365_resizer.c
 create mode 100644 drivers/staging/media/davinci_vpfe/dm365_resizer.h
 create mode 100644 drivers/staging/media/davinci_vpfe/vpfe.h
 create mode 100644 drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c
 create mode 100644 drivers/staging/media/davinci_vpfe/vpfe_mc_capture.h
 create mode 100644 drivers/staging/media/davinci_vpfe/vpfe_video.c
 create mode 100644 drivers/staging/media/davinci_vpfe/vpfe_video.h
  

Comments

Dan Carpenter Nov. 28, 2012, 11:45 a.m. UTC | #1
I wish people wouldn't submit big patches right before the merge
window opens...  :/ It's better to let it sit in linux-next for a
couple weeks so people can mess with it a bit.

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Hans Verkuil (hansverk) Nov. 28, 2012, 11:56 a.m. UTC | #2
On Wed 28 November 2012 12:45:37 Dan Carpenter wrote:
> I wish people wouldn't submit big patches right before the merge
> window opens...  :/ It's better to let it sit in linux-next for a
> couple weeks so people can mess with it a bit.

It's been under review for quite some time now, and the main change since
the last posted version is that this is now moved to staging/media.

So it is not yet ready for prime time, but we do want it in to simplify
the last remaining improvements needed to move it to drivers/media.

I'm happy with this going in given the circumstances.

Regards,

	Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Mauro Carvalho Chehab Nov. 28, 2012, 12:18 p.m. UTC | #3
Em Wed, 28 Nov 2012 12:56:10 +0100
Hans Verkuil <hansverk@cisco.com> escreveu:

> On Wed 28 November 2012 12:45:37 Dan Carpenter wrote:
> > I wish people wouldn't submit big patches right before the merge
> > window opens...  :/ It's better to let it sit in linux-next for a
> > couple weeks so people can mess with it a bit.
> 
> It's been under review for quite some time now, and the main change since
> the last posted version is that this is now moved to staging/media.
> 
> So it is not yet ready for prime time, but we do want it in to simplify
> the last remaining improvements needed to move it to drivers/media.

"last remaining improvements"? I didn't review the patchset, but
the TODO list seems to have several pending stuff there:

+- User space interface refinement
+        - Controls should be used when possible rather than private ioctl
+        - No enums should be used
+        - Use of MC and V4L2 subdev APIs when applicable
+        - Single interface header might suffice
+        - Current interface forces to configure everything at once
+- Get rid of the dm365_ipipe_hw.[ch] layer
+- Active external sub-devices defined by link configuration; no strcmp
+  needed
+- More generic platform data (i2c adapters)
+- The driver should have no knowledge of possible external subdevs; see
+  struct vpfe_subdev_id
+- Some of the hardware control should be refactorede
+- Check proper serialisation (through mutexes and spinlocks)
+- Names that are visible in kernel global namespace should have a common
+  prefix (or a few)

From the above comments, both Kernelspace and Userspace APIs require 
lots of work.

Also, it is not clear at all if this is a fork of the existing davinci
driver, or if it is a completely new driver for an already-supported
hardware, making very hard (if not impossible) to review it, and, if it
is yet-another-driver for the same hardware, moving it out of staging
will be a big issue, as it won't be trivial to check for regressions
introduced by a different driver.

> 
> I'm happy with this going in given the circumstances.

Well, I'm not.
  
Dan Carpenter Nov. 28, 2012, 12:22 p.m. UTC | #4
On Wed, Nov 28, 2012 at 12:56:10PM +0100, Hans Verkuil wrote:
> On Wed 28 November 2012 12:45:37 Dan Carpenter wrote:
> > I wish people wouldn't submit big patches right before the merge
> > window opens...  :/ It's better to let it sit in linux-next for a
> > couple weeks so people can mess with it a bit.
> 
> It's been under review for quite some time now, and the main change since
> the last posted version is that this is now moved to staging/media.
> 
> So it is not yet ready for prime time, but we do want it in to simplify
> the last remaining improvements needed to move it to drivers/media.
> 
> I'm happy with this going in given the circumstances.
> 

In the end this is just a driver, and I don't especially care.  But
it's like not just this one which makes me frustrated.  I really
believe in linux-next and I think everything should spend a couple
weeks there before being merged.

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Greg KH Nov. 28, 2012, 5:22 p.m. UTC | #5
On Wed, Nov 28, 2012 at 10:18:02AM -0200, Mauro Carvalho Chehab wrote:
> Em Wed, 28 Nov 2012 12:56:10 +0100
> Hans Verkuil <hansverk@cisco.com> escreveu:
> 
> > On Wed 28 November 2012 12:45:37 Dan Carpenter wrote:
> > > I wish people wouldn't submit big patches right before the merge
> > > window opens...  :/ It's better to let it sit in linux-next for a
> > > couple weeks so people can mess with it a bit.
> > 
> > It's been under review for quite some time now, and the main change since
> > the last posted version is that this is now moved to staging/media.
> > 
> > So it is not yet ready for prime time, but we do want it in to simplify
> > the last remaining improvements needed to move it to drivers/media.
> 
> "last remaining improvements"? I didn't review the patchset, but
> the TODO list seems to have several pending stuff there:
> 
> +- User space interface refinement
> +        - Controls should be used when possible rather than private ioctl
> +        - No enums should be used
> +        - Use of MC and V4L2 subdev APIs when applicable
> +        - Single interface header might suffice
> +        - Current interface forces to configure everything at once
> +- Get rid of the dm365_ipipe_hw.[ch] layer
> +- Active external sub-devices defined by link configuration; no strcmp
> +  needed
> +- More generic platform data (i2c adapters)
> +- The driver should have no knowledge of possible external subdevs; see
> +  struct vpfe_subdev_id
> +- Some of the hardware control should be refactorede
> +- Check proper serialisation (through mutexes and spinlocks)
> +- Names that are visible in kernel global namespace should have a common
> +  prefix (or a few)
> 
> From the above comments, both Kernelspace and Userspace APIs require 
> lots of work.
> 
> Also, it is not clear at all if this is a fork of the existing davinci
> driver, or if it is a completely new driver for an already-supported
> hardware, making very hard (if not impossible) to review it, and, if it
> is yet-another-driver for the same hardware, moving it out of staging
> will be a big issue, as it won't be trivial to check for regressions
> introduced by a different driver.
> 
> > 
> > I'm happy with this going in given the circumstances.
> 
> Well, I'm not.

Me either, it is way too late in the cycle to take huge stuff for 3.8,
sorry.

But as I don't manage drivers/staging/media/ there's not even anything I
can do here, it's Mauro's, so I'll leave it up to him :)

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Hans Verkuil Nov. 28, 2012, 7:18 p.m. UTC | #6
On Wed November 28 2012 18:22:48 Greg Kroah-Hartman wrote:
> On Wed, Nov 28, 2012 at 10:18:02AM -0200, Mauro Carvalho Chehab wrote:
> > Em Wed, 28 Nov 2012 12:56:10 +0100
> > Hans Verkuil <hansverk@cisco.com> escreveu:
> > 
> > > On Wed 28 November 2012 12:45:37 Dan Carpenter wrote:
> > > > I wish people wouldn't submit big patches right before the merge
> > > > window opens...  :/ It's better to let it sit in linux-next for a
> > > > couple weeks so people can mess with it a bit.
> > > 
> > > It's been under review for quite some time now, and the main change since
> > > the last posted version is that this is now moved to staging/media.
> > > 
> > > So it is not yet ready for prime time, but we do want it in to simplify
> > > the last remaining improvements needed to move it to drivers/media.
> > 
> > "last remaining improvements"? I didn't review the patchset, but
> > the TODO list seems to have several pending stuff there:
> > 
> > +- User space interface refinement
> > +        - Controls should be used when possible rather than private ioctl
> > +        - No enums should be used
> > +        - Use of MC and V4L2 subdev APIs when applicable
> > +        - Single interface header might suffice
> > +        - Current interface forces to configure everything at once
> > +- Get rid of the dm365_ipipe_hw.[ch] layer
> > +- Active external sub-devices defined by link configuration; no strcmp
> > +  needed
> > +- More generic platform data (i2c adapters)
> > +- The driver should have no knowledge of possible external subdevs; see
> > +  struct vpfe_subdev_id
> > +- Some of the hardware control should be refactorede
> > +- Check proper serialisation (through mutexes and spinlocks)
> > +- Names that are visible in kernel global namespace should have a common
> > +  prefix (or a few)
> > 
> > From the above comments, both Kernelspace and Userspace APIs require 
> > lots of work.

And that's why it is in staging. Should a long TODO list now suddenly
prevent staging from being used? In Barcelona we discussed this and the
only requirement we came up was was that it should compile.

> > Also, it is not clear at all if this is a fork of the existing davinci
> > driver, or if it is a completely new driver for an already-supported
> > hardware, making very hard (if not impossible) to review it, and, if it
> > is yet-another-driver for the same hardware, moving it out of staging
> > will be a big issue, as it won't be trivial to check for regressions
> > introduced by a different driver.

Having two competing drivers for the same hardware are hardly new. We've
had our share of those in the past, so I fail to see why this is suddenly
a problem or why that should prevent this driver from being merged in staging.

So once the driver is ready to be moved from staging to drivers/media, then
we can choose to keep the old driver around for awhile and let the user
make the choice which to use.

> > 
> > > 
> > > I'm happy with this going in given the circumstances.
> > 
> > Well, I'm not.
> 
> Me either, it is way too late in the cycle to take huge stuff for 3.8,
> sorry.
> 
> But as I don't manage drivers/staging/media/ there's not even anything I
> can do here, it's Mauro's, so I'll leave it up to him :)

It's a staging driver. So you know it will need more work and that it isn't
perfect. It's independent of any core kernel code, so it won't affect anything
else. I don't see why the size of the staging driver should matter in deciding
whether or not to include it. Nor do I see any reason why letting it linger
in linux-next for a few weeks should improve it in any way since we know it
has issues as it is in staging. I understand why linux-next is useful when
it comes to core code, but in this case it just seems to introduce a second
merge window before the 'real' merge window.

This is getting ridiculous in my opinion, not to mention discouraging for
developers. Staging is supposed to be fairly easy to get into (and out of,
if you don't continue development), but now I see all sorts of hurdles being
thrown up.

This driver has been in development for ages, it has seen quite a bit of
review resulting in the TODO list. I believe it's time to get this into
staging. If no further development takes place, then we just kick it out
in 6 months or so, no harm done.

Regards,

	Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Sylwester Nawrocki Nov. 28, 2012, 7:30 p.m. UTC | #7
On 11/28/2012 01:22 PM, Dan Carpenter wrote:
> In the end this is just a driver, and I don't especially care.  But
> it's like not just this one which makes me frustrated.  I really
> believe in linux-next and I think everything should spend a couple
> weeks there before being merged.

Couple of weeks in linux-next plus a couple of weeks of final patch
series version awaiting to being reviewed and picked up by a maintainer
makes almost entire kernel development cycle. These are huge additional
delays, especially in the embedded world. Things like these certainly
aren't encouraging for moving over from out-of-tree to the mainline
development process. And in this case we are talking only about merging
driver to the staging tree...

--

Thanks,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Greg KH Nov. 28, 2012, 7:30 p.m. UTC | #8
On Wed, Nov 28, 2012 at 08:18:20PM +0100, Hans Verkuil wrote:
> On Wed November 28 2012 18:22:48 Greg Kroah-Hartman wrote:
> > On Wed, Nov 28, 2012 at 10:18:02AM -0200, Mauro Carvalho Chehab wrote:
> > > Em Wed, 28 Nov 2012 12:56:10 +0100
> > > Hans Verkuil <hansverk@cisco.com> escreveu:
> > > 
> > > > On Wed 28 November 2012 12:45:37 Dan Carpenter wrote:
> > > > > I wish people wouldn't submit big patches right before the merge
> > > > > window opens...  :/ It's better to let it sit in linux-next for a
> > > > > couple weeks so people can mess with it a bit.
> > > > 
> > > > It's been under review for quite some time now, and the main change since
> > > > the last posted version is that this is now moved to staging/media.
> > > > 
> > > > So it is not yet ready for prime time, but we do want it in to simplify
> > > > the last remaining improvements needed to move it to drivers/media.
> > > 
> > > "last remaining improvements"? I didn't review the patchset, but
> > > the TODO list seems to have several pending stuff there:
> > > 
> > > +- User space interface refinement
> > > +        - Controls should be used when possible rather than private ioctl
> > > +        - No enums should be used
> > > +        - Use of MC and V4L2 subdev APIs when applicable
> > > +        - Single interface header might suffice
> > > +        - Current interface forces to configure everything at once
> > > +- Get rid of the dm365_ipipe_hw.[ch] layer
> > > +- Active external sub-devices defined by link configuration; no strcmp
> > > +  needed
> > > +- More generic platform data (i2c adapters)
> > > +- The driver should have no knowledge of possible external subdevs; see
> > > +  struct vpfe_subdev_id
> > > +- Some of the hardware control should be refactorede
> > > +- Check proper serialisation (through mutexes and spinlocks)
> > > +- Names that are visible in kernel global namespace should have a common
> > > +  prefix (or a few)
> > > 
> > > From the above comments, both Kernelspace and Userspace APIs require 
> > > lots of work.
> 
> And that's why it is in staging. Should a long TODO list now suddenly
> prevent staging from being used? In Barcelona we discussed this and the
> only requirement we came up was was that it should compile.

Yes, that's all I care about in staging, but as I stated, I don't
maintain drivers/staging/media/ that area is under Mauro's control
(MAINTAINERS even says this), and I'm a bit leery of going against the
wishes of an existing subsystem maintainer for adding staging drivers
that tie into their subsystem.

So if you get Mauro's approval, I'll be glad to queue it up.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Sakari Ailus Nov. 28, 2012, 8:04 p.m. UTC | #9
On Wed, Nov 28, 2012 at 12:56:10PM +0100, Hans Verkuil wrote:
> On Wed 28 November 2012 12:45:37 Dan Carpenter wrote:
> > I wish people wouldn't submit big patches right before the merge
> > window opens...  :/ It's better to let it sit in linux-next for a
> > couple weeks so people can mess with it a bit.
> 
> It's been under review for quite some time now, and the main change since
> the last posted version is that this is now moved to staging/media.
> 
> So it is not yet ready for prime time, but we do want it in to simplify
> the last remaining improvements needed to move it to drivers/media.
> 
> I'm happy with this going in given the circumstances.

My ack to that.
  
Greg KH Nov. 28, 2012, 8:46 p.m. UTC | #10
On Wed, Nov 28, 2012 at 08:30:04PM +0100, Sylwester Nawrocki wrote:
> On 11/28/2012 01:22 PM, Dan Carpenter wrote:
> > In the end this is just a driver, and I don't especially care.  But
> > it's like not just this one which makes me frustrated.  I really
> > believe in linux-next and I think everything should spend a couple
> > weeks there before being merged.
> 
> Couple of weeks in linux-next plus a couple of weeks of final patch
> series version awaiting to being reviewed and picked up by a maintainer
> makes almost entire kernel development cycle.

If I were to take this today, it would only live in linux-next for less
than a week before it would be sent to Linus due to where we are in the
development cycle, so Dan's objections are quite valid.

> These are huge additional delays, especially in the embedded world.

Embedded is special just like everyone else.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Dan Carpenter Nov. 28, 2012, 9:04 p.m. UTC | #11
On Wed, Nov 28, 2012 at 08:18:20PM +0100, Hans Verkuil wrote:
> but in this case it just seems to introduce a second
> merge window before the 'real' merge window.
> 

I don't care about this driver, but I mean yes.  That's the point of
linux-next.

I would say the standard rule is that it should sit in linux-next
for a week.  If I submitted a one line bugfix, normally that sits in
linux-next for a week before going upstream.

What frustrates me is that last merge window someone merged a driver
into linux-next for one day.  I submitted a memory corruption fix
the very next day but the maintainer said my fix arrived too late
for the 3.7 release.  What's the point of sending it to linux-next
if you don't have time to take feedback?  So yes, this thread is
mostly me still being cross about that.  :P

We were all expecting Linus to release the kernel last week so no
one should have expected to merge new drivers this week anyway.

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Dan Carpenter Nov. 28, 2012, 9:29 p.m. UTC | #12
On Wed, Nov 28, 2012 at 08:30:04PM +0100, Sylwester Nawrocki wrote:
> On 11/28/2012 01:22 PM, Dan Carpenter wrote:
> > In the end this is just a driver, and I don't especially care.  But
> > it's like not just this one which makes me frustrated.  I really
> > believe in linux-next and I think everything should spend a couple
> > weeks there before being merged.
> 
> Couple of weeks in linux-next plus a couple of weeks of final patch
> series version awaiting to being reviewed and picked up by a maintainer
> makes almost entire kernel development cycle. These are huge additional
> delays, especially in the embedded world. Things like these certainly
> aren't encouraging for moving over from out-of-tree to the mainline
> development process. And in this case we are talking only about merging
> driver to the staging tree...

Yeah.  A couple weeks is probably too long.  But I think a week is
totally reasonable.

You have the process wrong.  The maintainer reviews it first, merges
it into his -next git tree.  It sits in linux-next for a bit.  The
merge window opens up.  It is merged.  It gets tested for 3 months.
It is released.

It should work as a continuous even flow.  It shouldn't be a rush to
submit drivers right before the merge window opens.  It's not hard,
you can submit a driver to linux-next at any time.  It magically
flows through the process and is released some months later.

It does suck to add a 3 month delay for people who miss the cut off.
Don't wait until the last minute.  In the embedded world you can
use git cherry-pick to get the driver from linux-next.

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Sylwester Nawrocki Nov. 28, 2012, 11:47 p.m. UTC | #13
On 11/28/2012 10:29 PM, Dan Carpenter wrote:
> On Wed, Nov 28, 2012 at 08:30:04PM +0100, Sylwester Nawrocki wrote:
>> On 11/28/2012 01:22 PM, Dan Carpenter wrote:
>>> In the end this is just a driver, and I don't especially care.  But
>>> it's like not just this one which makes me frustrated.  I really
>>> believe in linux-next and I think everything should spend a couple
>>> weeks there before being merged.
>>
>> Couple of weeks in linux-next plus a couple of weeks of final patch
>> series version awaiting to being reviewed and picked up by a maintainer
>> makes almost entire kernel development cycle. These are huge additional
>> delays, especially in the embedded world. Things like these certainly
>> aren't encouraging for moving over from out-of-tree to the mainline
>> development process. And in this case we are talking only about merging
>> driver to the staging tree...
>
> Yeah.  A couple weeks is probably too long.  But I think a week is
> totally reasonable.

Agreed, exactly that couple weeks requirement seemed a bit long to me.

> You have the process wrong.  The maintainer reviews it first, merges
> it into his -next git tree.  It sits in linux-next for a bit.  The
> merge window opens up.  It is merged.  It gets tested for 3 months.
> It is released.

I believe what you're describing is true for most subsystems.  At
linux-media the process looks roughly that you prepare a patch and post
it to the mailing list for review.  Regular developers review it, you
address the comments and submit again.  Repeat these steps until
everyone's happy.  Then, when the patch looks like it is ready for
merging it is preferred to send the maintainer a pull request.
Now there can be a delay of up to couple weeks.  Afterwards the patch
in most cases gets merged, with a few possible change requests. However
it may happen the maintainer has different views on what's has been
agreed before and you start everything again.

With a few sub-maintainers recently appointed hopefully there can be
seen some improvement.

> It should work as a continuous even flow.  It shouldn't be a rush to
> submit drivers right before the merge window opens.  It's not hard,
> you can submit a driver to linux-next at any time.  It magically
> flows through the process and is released some months later.
>
> It does suck to add a 3 month delay for people who miss the cut off.
> Don't wait until the last minute.  In the embedded world you can
> use git cherry-pick to get the driver from linux-next.

Yeah, it's not unusual to work with specific -rc tree with multiple
subsystem -next branches on top of it.

It's just those cases where you're told to get feature A in the kernel
release X and it is already late in the development cycle... But it
might just be a matter of planning the work adequately with proper
understanding of the whole process.

--

Thanks,
Sylwester
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Hans Verkuil Nov. 29, 2012, 7:40 a.m. UTC | #14
On Thu November 29 2012 00:47:41 Sylwester Nawrocki wrote:
> On 11/28/2012 10:29 PM, Dan Carpenter wrote:
> > On Wed, Nov 28, 2012 at 08:30:04PM +0100, Sylwester Nawrocki wrote:
> >> On 11/28/2012 01:22 PM, Dan Carpenter wrote:
> >>> In the end this is just a driver, and I don't especially care.  But
> >>> it's like not just this one which makes me frustrated.  I really
> >>> believe in linux-next and I think everything should spend a couple
> >>> weeks there before being merged.
> >>
> >> Couple of weeks in linux-next plus a couple of weeks of final patch
> >> series version awaiting to being reviewed and picked up by a maintainer
> >> makes almost entire kernel development cycle. These are huge additional
> >> delays, especially in the embedded world. Things like these certainly
> >> aren't encouraging for moving over from out-of-tree to the mainline
> >> development process. And in this case we are talking only about merging
> >> driver to the staging tree...
> >
> > Yeah.  A couple weeks is probably too long.  But I think a week is
> > totally reasonable.
> 
> Agreed, exactly that couple weeks requirement seemed a bit long to me.
> 
> > You have the process wrong.  The maintainer reviews it first, merges
> > it into his -next git tree.  It sits in linux-next for a bit.  The
> > merge window opens up.  It is merged.  It gets tested for 3 months.
> > It is released.
> 
> I believe what you're describing is true for most subsystems.  At
> linux-media the process looks roughly that you prepare a patch and post
> it to the mailing list for review.  Regular developers review it, you
> address the comments and submit again.  Repeat these steps until
> everyone's happy.  Then, when the patch looks like it is ready for
> merging it is preferred to send the maintainer a pull request.
> Now there can be a delay of up to couple weeks.  Afterwards the patch
> in most cases gets merged, with a few possible change requests. However
> it may happen the maintainer has different views on what's has been
> agreed before and you start everything again.

Yes, and the problem is that the maintainer (Mauro) tends to look at this
close to the merge window, generally leaving you with very little time to
make adjustments.

In all honesty, in this particular case the pull request came in late
because other reviewers asked TI to move the driver to staging because we
thought the driver wasn't quite ready yet.

If it would just be merged as a staging driver, then it would most likely
be sitting in linux-next by now since a few days, well in time for the merge
window. Instead we are having this whole discussion :-(

> With a few sub-maintainers recently appointed hopefully there can be
> seen some improvement.

I sincerely hope so. Of course, since I'm going to be one of the sub-maintainers
I'm going to see this from the other side as well, so I might get the same
critisism in the future :-)

> > It should work as a continuous even flow.  It shouldn't be a rush to
> > submit drivers right before the merge window opens.  It's not hard,
> > you can submit a driver to linux-next at any time.  It magically
> > flows through the process and is released some months later.
> >
> > It does suck to add a 3 month delay for people who miss the cut off.
> > Don't wait until the last minute.  In the embedded world you can
> > use git cherry-pick to get the driver from linux-next.
> 
> Yeah, it's not unusual to work with specific -rc tree with multiple
> subsystem -next branches on top of it.
> 
> It's just those cases where you're told to get feature A in the kernel
> release X and it is already late in the development cycle... But it
> might just be a matter of planning the work adequately with proper
> understanding of the whole process.

Well, it would work if you can rely on patches being reviewed and merged
in a timely manner. Hopefully that will happen when the sub-maintainers
can start next year. It should make everything more predictable.

Regards,

	Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Hans Verkuil Nov. 29, 2012, 7:43 a.m. UTC | #15
On Wed November 28 2012 20:30:21 Greg Kroah-Hartman wrote:
> On Wed, Nov 28, 2012 at 08:18:20PM +0100, Hans Verkuil wrote:
> > On Wed November 28 2012 18:22:48 Greg Kroah-Hartman wrote:
> > > On Wed, Nov 28, 2012 at 10:18:02AM -0200, Mauro Carvalho Chehab wrote:
> > > > Em Wed, 28 Nov 2012 12:56:10 +0100
> > > > Hans Verkuil <hansverk@cisco.com> escreveu:
> > > > 
> > > > > On Wed 28 November 2012 12:45:37 Dan Carpenter wrote:
> > > > > > I wish people wouldn't submit big patches right before the merge
> > > > > > window opens...  :/ It's better to let it sit in linux-next for a
> > > > > > couple weeks so people can mess with it a bit.
> > > > > 
> > > > > It's been under review for quite some time now, and the main change since
> > > > > the last posted version is that this is now moved to staging/media.
> > > > > 
> > > > > So it is not yet ready for prime time, but we do want it in to simplify
> > > > > the last remaining improvements needed to move it to drivers/media.
> > > > 
> > > > "last remaining improvements"? I didn't review the patchset, but
> > > > the TODO list seems to have several pending stuff there:
> > > > 
> > > > +- User space interface refinement
> > > > +        - Controls should be used when possible rather than private ioctl
> > > > +        - No enums should be used
> > > > +        - Use of MC and V4L2 subdev APIs when applicable
> > > > +        - Single interface header might suffice
> > > > +        - Current interface forces to configure everything at once
> > > > +- Get rid of the dm365_ipipe_hw.[ch] layer
> > > > +- Active external sub-devices defined by link configuration; no strcmp
> > > > +  needed
> > > > +- More generic platform data (i2c adapters)
> > > > +- The driver should have no knowledge of possible external subdevs; see
> > > > +  struct vpfe_subdev_id
> > > > +- Some of the hardware control should be refactorede
> > > > +- Check proper serialisation (through mutexes and spinlocks)
> > > > +- Names that are visible in kernel global namespace should have a common
> > > > +  prefix (or a few)
> > > > 
> > > > From the above comments, both Kernelspace and Userspace APIs require 
> > > > lots of work.
> > 
> > And that's why it is in staging. Should a long TODO list now suddenly
> > prevent staging from being used? In Barcelona we discussed this and the
> > only requirement we came up was was that it should compile.
> 
> Yes, that's all I care about in staging, but as I stated, I don't
> maintain drivers/staging/media/ that area is under Mauro's control
> (MAINTAINERS even says this), and I'm a bit leery of going against the
> wishes of an existing subsystem maintainer for adding staging drivers
> that tie into their subsystem.
> 
> So if you get Mauro's approval, I'll be glad to queue it up.

Just for the record, my comments were addressed to Mauro, not to you.
I should have stated that in my mail explicitly. As you said, it's
Mauro's decision.

Regards,

	Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Laurent Pinchart Nov. 29, 2012, 10:12 a.m. UTC | #16
On Wednesday 28 November 2012 16:12:00 Prabhakar Lad wrote:
> From: Manjunath Hadli <manjunath.hadli@ti.com>

For staging, and provided that all parties involved understand that an API 
compatibility layer with the existing drivers/media/platform/davinci/ driver 
(called the "existing driver") will need to be provided when this media 
controller aware driver will move out of staging to drivers/media/ and replace 
the existing driver,

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> Mauro/Greg,
>  The below series of patches have gone through good amount of reviews, and
> agreed by Laurent, Hans and Sakari to be part of the staging tree. I am
> combining the patchs with the pull request so we can get them into the 3.8
> kernel. Please pull these patches.If you want a seperate pull request,
> please let me know.
> 
> This patch set adds media controller based capture driver for
> DM365.
> 
> This driver bases its design on Laurent Pinchart's Media Controller Design
> whose patches for Media Controller and subdev enhancements form the base.
> The driver also takes copious elements taken from Laurent Pinchart and
> others' OMAP ISP driver based on Media Controller. So thank you all the
> people who are responsible for the Media Controller and the OMAP ISP driver.
> 
> Also, the core functionality of the driver comes from the arago vpfe capture
> driver of which the isif capture was based on V4L2, with other drivers like
> ipipe, ipipeif and Resizer.
> 
> Changes for v2:
> 1: Migrated the driver for videobuf2 usage pointed Hans.
> 2: Changed the design as pointed by Laurent, Exposed one more subdevs
>    ipipeif and split the resizer subdev into three subdevs.
> 3: Rearrganed the patch sequence and changed the commit messages.
> 4: Changed the file architecture as pointed by Laurent.
> 
> Changes for v3:
> 1: Rebased on staging.
> 2: Seprated out patches which would go into staging.
> 
> The following changes since commit c6c22955f80f2db9614b01fe5a3d1cfcd8b3d848:
> 
>   [media] dma-mapping: fix dma_common_get_sgtable() conditional compilation
> (2012-11-27 09:42:31 -0200)
> 
> are available in the git repository at:
>   git://linuxtv.org/mhadli/v4l-dvb-davinci_devices.git vpfe_driver_staging
> 
> Manjunath Hadli (9):
>       davinci: vpfe: add v4l2 capture driver with media interface
>       davinci: vpfe: add v4l2 video driver support
>       davinci: vpfe: dm365: add IPIPEIF driver based on media framework
>       davinci: vpfe: dm365: add ISIF driver based on media framework
>       davinci: vpfe: dm365: add IPIPE support for media controller driver
>       davinci: vpfe: dm365: add IPIPE hardware layer support
>       davinci: vpfe: dm365: resizer driver based on media framework
>       davinci: vpfe: dm365: add build infrastructure for capture driver
>       davinci: vpfe: Add documentation and TODO
> 
>  drivers/staging/media/Kconfig                      |    2 +
>  drivers/staging/media/Makefile                     |    1 +
>  drivers/staging/media/davinci_vpfe/Kconfig         |    9 +
>  drivers/staging/media/davinci_vpfe/Makefile        |    3 +
>  drivers/staging/media/davinci_vpfe/TODO            |   34 +
>  .../staging/media/davinci_vpfe/davinci-vpfe-mc.txt |  154 ++
>  .../staging/media/davinci_vpfe/davinci_vpfe_user.h | 1290 ++++++++++++
>  drivers/staging/media/davinci_vpfe/dm365_ipipe.c   | 1863 +++++++++++++++++
> drivers/staging/media/davinci_vpfe/dm365_ipipe.h   |  179 ++
>  .../staging/media/davinci_vpfe/dm365_ipipe_hw.c    | 1048 ++++++++++
>  .../staging/media/davinci_vpfe/dm365_ipipe_hw.h    |  559 ++++++
>  drivers/staging/media/davinci_vpfe/dm365_ipipeif.c | 1071 ++++++++++
>  drivers/staging/media/davinci_vpfe/dm365_ipipeif.h |  233 +++
>  .../media/davinci_vpfe/dm365_ipipeif_user.h        |   93 +
>  drivers/staging/media/davinci_vpfe/dm365_isif.c    | 2104 +++++++++++++++++
>  drivers/staging/media/davinci_vpfe/dm365_isif.h    |  203 ++
>  .../staging/media/davinci_vpfe/dm365_isif_regs.h   |  294 +++
>  drivers/staging/media/davinci_vpfe/dm365_resizer.c | 1999 +++++++++++++++++
>  drivers/staging/media/davinci_vpfe/dm365_resizer.h |  244 +++
>  drivers/staging/media/davinci_vpfe/vpfe.h          |   86 +
>  .../staging/media/davinci_vpfe/vpfe_mc_capture.c   |  740 +++++++
>  .../staging/media/davinci_vpfe/vpfe_mc_capture.h   |   97 +
>  drivers/staging/media/davinci_vpfe/vpfe_video.c    | 1620 +++++++++++++++
>  drivers/staging/media/davinci_vpfe/vpfe_video.h    |  155 ++
>  24 files changed, 14081 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/staging/media/davinci_vpfe/Kconfig
>  create mode 100644 drivers/staging/media/davinci_vpfe/Makefile
>  create mode 100644 drivers/staging/media/davinci_vpfe/TODO
>  create mode 100644 drivers/staging/media/davinci_vpfe/davinci-vpfe-mc.txt
>  create mode 100644 drivers/staging/media/davinci_vpfe/davinci_vpfe_user.h
>  create mode 100644 drivers/staging/media/davinci_vpfe/dm365_ipipe.c
>  create mode 100644 drivers/staging/media/davinci_vpfe/dm365_ipipe.h
>  create mode 100644 drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c
>  create mode 100644 drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.h
>  create mode 100644 drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
>  create mode 100644 drivers/staging/media/davinci_vpfe/dm365_ipipeif.h
>  create mode 100644 drivers/staging/media/davinci_vpfe/dm365_ipipeif_user.h
>  create mode 100644 drivers/staging/media/davinci_vpfe/dm365_isif.c
>  create mode 100644 drivers/staging/media/davinci_vpfe/dm365_isif.h
>  create mode 100644 drivers/staging/media/davinci_vpfe/dm365_isif_regs.h
>  create mode 100644 drivers/staging/media/davinci_vpfe/dm365_resizer.c
>  create mode 100644 drivers/staging/media/davinci_vpfe/dm365_resizer.h
>  create mode 100644 drivers/staging/media/davinci_vpfe/vpfe.h
>  create mode 100644 drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c
>  create mode 100644 drivers/staging/media/davinci_vpfe/vpfe_mc_capture.h
>  create mode 100644 drivers/staging/media/davinci_vpfe/vpfe_video.c
>  create mode 100644 drivers/staging/media/davinci_vpfe/vpfe_video.h
  
Mauro Carvalho Chehab Nov. 29, 2012, 10:39 a.m. UTC | #17
Em Thu, 29 Nov 2012 08:43:36 +0100
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On Wed November 28 2012 20:30:21 Greg Kroah-Hartman wrote:
> > On Wed, Nov 28, 2012 at 08:18:20PM +0100, Hans Verkuil wrote:
> > > On Wed November 28 2012 18:22:48 Greg Kroah-Hartman wrote:
> > > > On Wed, Nov 28, 2012 at 10:18:02AM -0200, Mauro Carvalho Chehab wrote:
> > > > > Em Wed, 28 Nov 2012 12:56:10 +0100
> > > > > Hans Verkuil <hansverk@cisco.com> escreveu:
> > > > > 
> > > > > > On Wed 28 November 2012 12:45:37 Dan Carpenter wrote:
> > > > > > > I wish people wouldn't submit big patches right before the merge
> > > > > > > window opens...  :/ It's better to let it sit in linux-next for a
> > > > > > > couple weeks so people can mess with it a bit.
> > > > > > 
> > > > > > It's been under review for quite some time now, and the main change since
> > > > > > the last posted version is that this is now moved to staging/media.
> > > > > > 
> > > > > > So it is not yet ready for prime time, but we do want it in to simplify
> > > > > > the last remaining improvements needed to move it to drivers/media.
> > > > > 
> > > > > "last remaining improvements"? I didn't review the patchset, but
> > > > > the TODO list seems to have several pending stuff there:
> > > > > 
> > > > > +- User space interface refinement
> > > > > +        - Controls should be used when possible rather than private ioctl
> > > > > +        - No enums should be used
> > > > > +        - Use of MC and V4L2 subdev APIs when applicable
> > > > > +        - Single interface header might suffice
> > > > > +        - Current interface forces to configure everything at once
> > > > > +- Get rid of the dm365_ipipe_hw.[ch] layer
> > > > > +- Active external sub-devices defined by link configuration; no strcmp
> > > > > +  needed
> > > > > +- More generic platform data (i2c adapters)
> > > > > +- The driver should have no knowledge of possible external subdevs; see
> > > > > +  struct vpfe_subdev_id
> > > > > +- Some of the hardware control should be refactorede
> > > > > +- Check proper serialisation (through mutexes and spinlocks)
> > > > > +- Names that are visible in kernel global namespace should have a common
> > > > > +  prefix (or a few)
> > > > > 
> > > > > From the above comments, both Kernelspace and Userspace APIs require 
> > > > > lots of work.
> > > 
> > > And that's why it is in staging. Should a long TODO list now suddenly
> > > prevent staging from being used? In Barcelona we discussed this and the
> > > only requirement we came up was was that it should compile.
> > 
> > Yes, that's all I care about in staging, but as I stated, I don't
> > maintain drivers/staging/media/ that area is under Mauro's control
> > (MAINTAINERS even says this), and I'm a bit leery of going against the
> > wishes of an existing subsystem maintainer for adding staging drivers
> > that tie into their subsystem.

On my understanding, this is not a "normal" staging driver for some
unsupported hardware. It is a driver that is meant to replace an 
existing driver, whose plans is to implement a different, userspace-incompatible
API set than the existing one. In other words, merging it as-is would give the
false impression that only solving the TODO items would be enough to promote it.

However, the main criteria for a replacement driver is to not cause any
regression. So, a compatibility layer and compatibility tests that warrants
this is a requirement (eventually using the  libv4l's LD_PRELOAD approach). 

So, if the ones working on the driver are also willing to work on the 
needed compatibility bits, I'm ok on merging it at staging; if not, there's
no sense on spending everybody's time on something that will be discarded
on a few kernel cycles.

So, the main discussion here is not about merging it at staging or not; is
to be sure that, once it got merged, the right things will be addressed,
in order to allow it to replace the existing driver.

Of course, Dan's request is valid; pushing it right now at -next will give
only a week for the others to review a big driver. So, better to delay it.

In any case, there are already 400+ patches on my queue that arrived
before these late-submitted patch series. So, it would be delayed
anyway to the next cycle, due to the very high volume of pending stuff
there, and to his late submission.

So, in summary:

Prabhakar/Manju:

If you'll be committed to make sure that no regressions will happen when this
driver will be promoted and replace the existing driver, please update
the TODO to point that the compatibility bits is needed.

I'll then merge it after the end of the merge window.

Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Manjunath Hadli Nov. 29, 2012, 12:45 p.m. UTC | #18
Hi Mauro,

On Thursday 29 November 2012 04:09 PM, Mauro Carvalho Chehab wrote:
> Em Thu, 29 Nov 2012 08:43:36 +0100
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> 
>> On Wed November 28 2012 20:30:21 Greg Kroah-Hartman wrote:
>>> On Wed, Nov 28, 2012 at 08:18:20PM +0100, Hans Verkuil wrote:
>>>> On Wed November 28 2012 18:22:48 Greg Kroah-Hartman wrote:
>>>>> On Wed, Nov 28, 2012 at 10:18:02AM -0200, Mauro Carvalho Chehab wrote:
>>>>>> Em Wed, 28 Nov 2012 12:56:10 +0100
>>>>>> Hans Verkuil <hansverk@cisco.com> escreveu:
>>>>>>
>>>>>>> On Wed 28 November 2012 12:45:37 Dan Carpenter wrote:
>>>>>>>> I wish people wouldn't submit big patches right before the merge
>>>>>>>> window opens...  :/ It's better to let it sit in linux-next for a
>>>>>>>> couple weeks so people can mess with it a bit.
>>>>>>>
>>>>>>> It's been under review for quite some time now, and the main change since
>>>>>>> the last posted version is that this is now moved to staging/media.
>>>>>>>
>>>>>>> So it is not yet ready for prime time, but we do want it in to simplify
>>>>>>> the last remaining improvements needed to move it to drivers/media.
>>>>>>
>>>>>> "last remaining improvements"? I didn't review the patchset, but
>>>>>> the TODO list seems to have several pending stuff there:
>>>>>>
>>>>>> +- User space interface refinement
>>>>>> +        - Controls should be used when possible rather than private ioctl
>>>>>> +        - No enums should be used
>>>>>> +        - Use of MC and V4L2 subdev APIs when applicable
>>>>>> +        - Single interface header might suffice
>>>>>> +        - Current interface forces to configure everything at once
>>>>>> +- Get rid of the dm365_ipipe_hw.[ch] layer
>>>>>> +- Active external sub-devices defined by link configuration; no strcmp
>>>>>> +  needed
>>>>>> +- More generic platform data (i2c adapters)
>>>>>> +- The driver should have no knowledge of possible external subdevs; see
>>>>>> +  struct vpfe_subdev_id
>>>>>> +- Some of the hardware control should be refactorede
>>>>>> +- Check proper serialisation (through mutexes and spinlocks)
>>>>>> +- Names that are visible in kernel global namespace should have a common
>>>>>> +  prefix (or a few)
>>>>>>
>>>>>> From the above comments, both Kernelspace and Userspace APIs require 
>>>>>> lots of work.
>>>>
>>>> And that's why it is in staging. Should a long TODO list now suddenly
>>>> prevent staging from being used? In Barcelona we discussed this and the
>>>> only requirement we came up was was that it should compile.
>>>
>>> Yes, that's all I care about in staging, but as I stated, I don't
>>> maintain drivers/staging/media/ that area is under Mauro's control
>>> (MAINTAINERS even says this), and I'm a bit leery of going against the
>>> wishes of an existing subsystem maintainer for adding staging drivers
>>> that tie into their subsystem.
> 
> On my understanding, this is not a "normal" staging driver for some
> unsupported hardware. It is a driver that is meant to replace an 
> existing driver, whose plans is to implement a different, userspace-incompatible
> API set than the existing one. In other words, merging it as-is would give the
> false impression that only solving the TODO items would be enough to promote it.
> 
> However, the main criteria for a replacement driver is to not cause any
> regression. So, a compatibility layer and compatibility tests that warrants
> this is a requirement (eventually using the  libv4l's LD_PRELOAD approach). 
> 
> So, if the ones working on the driver are also willing to work on the 
> needed compatibility bits, I'm ok on merging it at staging; if not, there's
> no sense on spending everybody's time on something that will be discarded
> on a few kernel cycles.
> 
> So, the main discussion here is not about merging it at staging or not; is
> to be sure that, once it got merged, the right things will be addressed,
> in order to allow it to replace the existing driver.
> 
> Of course, Dan's request is valid; pushing it right now at -next will give
> only a week for the others to review a big driver. So, better to delay it.
> 
> In any case, there are already 400+ patches on my queue that arrived
> before these late-submitted patch series. So, it would be delayed
> anyway to the next cycle, due to the very high volume of pending stuff
> there, and to his late submission.
> 
> So, in summary:
> 
> Prabhakar/Manju:
> 
> If you'll be committed to make sure that no regressions will happen when this
> driver will be promoted and replace the existing driver, please update
> the TODO to point that the compatibility bits is needed.
Sure Mauro. All this long wait to get into the mainline would be for
nothing if we do not go ahead and make the necessary changes you want us
to do to get to the media folder. We will update the TODO patch and
resend the series.
> 
> I'll then merge it after the end of the merge window.
thank you very much.
> 
> Regards,
> Mauro
> 
Thanks and Regards,
-Manju

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Mauro Carvalho Chehab Nov. 29, 2012, 4:38 p.m. UTC | #19
Em Thu, 29 Nov 2012 18:15:39 +0530
Manjunath Hadli <manjunath.hadli@ti.com> escreveu:

> > So, in summary:
> > 
> > Prabhakar/Manju:
> > 
> > If you'll be committed to make sure that no regressions will happen when this
> > driver will be promoted and replace the existing driver, please update
> > the TODO to point that the compatibility bits is needed.
> Sure Mauro. All this long wait to get into the mainline would be for
> nothing if we do not go ahead and make the necessary changes you want us
> to do to get to the media folder. We will update the TODO patch and
> resend the series.

Ok, thank you!

Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
  
Sakari Ailus Nov. 30, 2012, 9:47 a.m. UTC | #20
On Wed, Nov 28, 2012 at 04:12:00PM +0530, Prabhakar Lad wrote:
> From: Manjunath Hadli <manjunath.hadli@ti.com>
> 
> Mauro/Greg,
>  The below series of patches have gone through good amount of reviews, and
> agreed by Laurent, Hans and Sakari to be part of the staging tree. I am combining
> the patchs with the pull request so we can get them into the 3.8 kernel.
> Please pull these patches.If you want a seperate pull request, please let me
> know.
> 
> This patch set adds media controller based capture driver for
> DM365.

For the whole set --- granted that the TODO item to "add support for regular
V4L2 applications through user space libraries" is added:

Acked-by: Sakari Ailus <sakari.ailus@iki.fi>
  
Hans Verkuil Nov. 30, 2012, 9:54 a.m. UTC | #21
On Fri November 30 2012 10:47:39 Sakari Ailus wrote:
> On Wed, Nov 28, 2012 at 04:12:00PM +0530, Prabhakar Lad wrote:
> > From: Manjunath Hadli <manjunath.hadli@ti.com>
> > 
> > Mauro/Greg,
> >  The below series of patches have gone through good amount of reviews, and
> > agreed by Laurent, Hans and Sakari to be part of the staging tree. I am combining
> > the patchs with the pull request so we can get them into the 3.8 kernel.
> > Please pull these patches.If you want a seperate pull request, please let me
> > know.
> > 
> > This patch set adds media controller based capture driver for
> > DM365.
> 
> For the whole set --- granted that the TODO item to "add support for regular
> V4L2 applications through user space libraries" is added:
> 
> Acked-by: Sakari Ailus <sakari.ailus@iki.fi>
> 
> 

Ditto for the TODO item.

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html