Re: [PATCH v3 0/9] Media Controller capture driver for DM365
On Wed, Nov 28, 2012 at 04:12:00PM +0530, Prabhakar Lad wrote: From: Manjunath Hadli manjunath.ha...@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.ai...@iki.fi -- Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/9] Media Controller capture driver for DM365
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.ha...@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.ai...@iki.fi Ditto for the TODO item. Acked-by: Hans Verkuil hans.verk...@cisco.com -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/9] Media Controller capture driver for DM365
On Wednesday 28 November 2012 16:12:00 Prabhakar Lad wrote: From: Manjunath Hadli manjunath.ha...@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.pinch...@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
Re: [PATCH v3 0/9] Media Controller capture driver for DM365
Em Thu, 29 Nov 2012 08:43:36 +0100 Hans Verkuil hverk...@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 hansv...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/9] Media Controller capture driver for DM365
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 hverk...@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 hansv...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/9] Media Controller capture driver for DM365
Em Thu, 29 Nov 2012 18:15:39 +0530 Manjunath Hadli manjunath.ha...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/9] Media Controller capture driver for DM365
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/9] Media Controller capture driver for DM365
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/9] Media Controller capture driver for DM365
Em Wed, 28 Nov 2012 12:56:10 +0100 Hans Verkuil hansv...@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. -- Regards, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/9] Media Controller capture driver for DM365
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/9] Media Controller capture driver for DM365
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 hansv...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/9] Media Controller capture driver for DM365
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 hansv...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/9] Media Controller capture driver for DM365
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 hansv...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/9] Media Controller capture driver for DM365
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/9] Media Controller capture driver for DM365
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. -- Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/9] Media Controller capture driver for DM365
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/9] Media Controller capture driver for DM365
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/9] Media Controller capture driver for DM365
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/9] Media Controller capture driver for DM365
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/9] Media Controller capture driver for DM365
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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/9] Media Controller capture driver for DM365
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 hansv...@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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
