Re: RFC: First draft of guidelines for submitting patches to linux-media
Em Tue, 11 Dec 2012 00:52:39 +0100 Laurent Pinchart laurent.pinch...@ideasonboard.com escreveu: Hi, On Monday 10 December 2012 16:33:13 Mauro Carvalho Chehab wrote: Em Mon, 10 Dec 2012 14:07:09 +0100 Hans Verkuil escreveu: Hi all, As discussed in Barcelona I would write a text describing requirements for new drivers and what to expect when submitting patches to linux-media. This is a first rough draft and nothing is fixed yet. I have a few open questions: 1) Where to put it? Maybe at media-build.git. Not everybody uses (or is even aware of) media-build. The goal here is to make sure that new driver developers will run into the guidelines before then spend months writing code that can't be upstreamed. Documentation/ thus looks like a good place to me. It might be a good idea to add a reference to the guidelines in the API DocBook documentation, regardless of where the guidelines end up being stored. If a developer reads a single document only it will probably be the API reference. Agreed. I'm thinking on putting there, under devel_contrib, the main scripts I use here to handle patches. /me needs some time to sanitize them and add there. One thing I would propose that we improve is to move the dvb and video4linux directories in Documentation/ to Documentation/media to correctly reflect the drivers/media structure. If we do that, then we can put this document in Documentation/media/SubmittingMediaPatches. Hmm... I don't see any other subsystems having their own document for that. We may need to discuss it upstream before doing that, and be prepared to answer why we thing sub-systems would need their own rules there. Things like requiring the use of the control framework is obviously V4L- specific, we can't add that to Documentation/SubmittingPatches :-) In any case, I think that the better is to store it at media-build.git tree, and later open such discussions upstream, if we think it is valuable enough. Alternatively, this is something we can document in our wiki. 2) Are there DVB requirements as well for new drivers? We discussed a list of V4L2 requirements in Barcelona, but I wonder if there is a similar list that can be made for DVB drivers. Input on that will be welcome. See below. 3) This document describes the situation we will have when the submaintainers take their place early next year. So please check if I got that part right. 4) In Barcelona we discussed 'tags' for patches to help organize them. I've made a proposal for those in this document. Feedback is very welcome. 5) As discussed in Barcelona there will be git tree maintainers for specific platforms, but we didn't really go into detail who would be responsible for which platform. If you want to maintain a particular platform, then please let me know. 6) The patchwork section is very short at the moment. It should be extended when patchwork gets support to recognize the various tags. 7) Anything else that should be discussed here? Again, remember that this is a rough draft only, so be gentle with me :-) Regards, Hans --- cut here --- General Information === For general information on how to submit patches see: http://linuxtv.org/wiki/index.php/Developer_Section In particular the section 'Submitting Your Work'. This document goes into more detail regarding media specific requirements when submitting patches and what the patch flow looks like in this subsystem. I think we should add a paragraph here saying that rules may have exceptions, when there's a clear reason why a certain submission should need a different criteria. I agree. For instance the uvcvideo driver doesn't use the control framework, and has good reasons not to. This must be the exception rather than the rule, but we might have more than one exception. Yes. Also, IMHO, we should add a notice that this list is not exhaustive, and may be changed, keeping it for at least one or two Kernel cycles, while it doesn't get proofed/matured, as I'm sure we'll forget things. Submitting New Media Drivers When submitting new media drivers for inclusion in drivers/staging/media all that is required is that the driver compiles with the latest kernel and that an entry is added to the MAINTAINERS file Please add: and what is missing there for it to be promoted to be a main driver is documented at the TODO file. It should be noticed, however, that it is expected that the driver will be fixed to fulfill the requirements for upstream addition. If a driver at staging lacks relevant patches fixing it for more than a kernel cycle, it can be dropped without
Re: RFC: First draft of guidelines for submitting patches to linux-media
Hi Mauro, On Tuesday 11 December 2012 08:29:30 Mauro Carvalho Chehab wrote: Em Tue, 11 Dec 2012 00:52:39 +0100 Laurent Pinchart escreveu: On Monday 10 December 2012 16:33:13 Mauro Carvalho Chehab wrote: Em Mon, 10 Dec 2012 14:07:09 +0100 Hans Verkuil escreveu: [snip] Submitting New Media Drivers When submitting new media drivers for inclusion in drivers/staging/media all that is required is that the driver compiles with the latest kernel and that an entry is added to the MAINTAINERS file Please add: and what is missing there for it to be promoted to be a main driver is documented at the TODO file. It should be noticed, however, that it is expected that the driver will be fixed to fulfill the requirements for upstream addition. If a driver at staging lacks relevant patches fixing it for more than a kernel cycle, it can be dropped without further notice. Maybe a single kernel cycle is a bit too strict. The unexpected can happen, so let's not be too harsh. The above is not saying that it should be fixed on one kernel cycle. It says, instead, that drivers without relevant changes during a cycle can be dropped. We'll likely not enforce it too hard, except if we notice some sort of bad faith from the driver's maintainer. That's my point, exactly. The text above just sounds a bit too harsh for my taste, it might even scare people :-) I of course share your point of view that we want developers to understand that they need to work on staging drivers and not let them rot, And I'm pretty sure we'll always send a notice. The notice will likely be just a patch to the ML c/c the driver's maintainer and the mailing list. As the driver's maintainer email's address might have changed, and/or he might not be subscribed at the ML, it may happen that such email will never reach him. So, the it can be dropped without further notice wording is meant to avoid later complains that from driver's maintainer that he was not warned. It also passes the idea that no ack from the driver's maintainer is needed/expected on such patch. If you think it is badly written, feel free to change it, but keeping this idea. What about If no real effort to get a driver out of staging is noticed, the driver can be dropped from the kernel altogether. This policy can be applied over a single kernel release period and without any notice, although we will try our best to communicate with the driver developers and not to enforce the policy too harshly. For inclusion as a non-staging driver the requirements are more strict: General requirements: - It must pass checkpatch.pl, but see the note regarding interpreting the output from checkpatch below. - An entry for the driver is added to the MAINTAINERS file. Please add: - Properly use the kernel internal APIs; - Should re-invent the wheel, by adding new defines, math logic, etc that already exists in the Kernel; Should *not* ? :-) Gah... Yeah, it should read, instead: shouldn't. - Errors should be reported as negative numbers, using the Kernel error codes; CodingStyle, chapter 16 (although not as clearly stated). Yes, I know. Yet, this is one of the most frequent bad things we notice on code from new developers. IMHO, it doesn't hurt to explicitly say it here, likely pointing to the CodingStyle corresponding chapter. Maybe we could just add Follow the CodingStyle guidelines. - typedefs should't be used; CodingStyle, chapter 5. Same as above: that's the second most frequent bad thing. It seems that windows-way is to create typedefs for each struct and return positive, driver-specific return codes. At least I saw the very same pattern on all windows drivers I looked. [snip] Timeline for code submissions = After a new kernel is released the merge window will be open for about two weeks Please add: for the maintainers to send him the patches they already received him ? I suppose you mean Linus :-) Yes. Maybe it can be changed from send him to send upstream. during the last development cycle, and that went into the linux-next tree in time for the other maintainers and reviewers to double-check the entire set of changes for the next Linux version. (yeah, I know you're talking more about it later, but I think this makes it a little clearer that no submissions will typically be accepted so late at the development cycle). During that time Linus will merge any pending work for the next kernel. Once that merge window is closed only regression fixes and serious bug fixes will be accepted, everything else will be postponed please add: upstream postponed upstream ? What do you mean ? This is just a small
Re: RFC: First draft of guidelines for submitting patches to linux-media
I've added a few quick remarks below. I'll collect all the very useful feedback on Friday and post a new version. After that I'm on vacation for three weeks. On Tue 11 December 2012 11:29:30 Mauro Carvalho Chehab wrote: Em Tue, 11 Dec 2012 00:52:39 +0100 Laurent Pinchart laurent.pinch...@ideasonboard.com escreveu: Hi, On Monday 10 December 2012 16:33:13 Mauro Carvalho Chehab wrote: Em Mon, 10 Dec 2012 14:07:09 +0100 Hans Verkuil escreveu: Hi all, As discussed in Barcelona I would write a text describing requirements for new drivers and what to expect when submitting patches to linux-media. This is a first rough draft and nothing is fixed yet. I have a few open questions: 1) Where to put it? Maybe at media-build.git. Not everybody uses (or is even aware of) media-build. The goal here is to make sure that new driver developers will run into the guidelines before then spend months writing code that can't be upstreamed. Documentation/ thus looks like a good place to me. I agree with Laurent here. The media_build repo isn't widely used, certainly not in the embedded world. I don't think we should stick documentation there. It might be a good idea to add a reference to the guidelines in the API DocBook documentation, regardless of where the guidelines end up being stored. If a developer reads a single document only it will probably be the API reference. Agreed. I'm thinking on putting there, under devel_contrib, the main scripts I use here to handle patches. /me needs some time to sanitize them and add there. One thing I would propose that we improve is to move the dvb and video4linux directories in Documentation/ to Documentation/media to correctly reflect the drivers/media structure. If we do that, then we can put this document in Documentation/media/SubmittingMediaPatches. Hmm... I don't see any other subsystems having their own document for that. We may need to discuss it upstream before doing that, and be prepared to answer why we thing sub-systems would need their own rules there. Things like requiring the use of the control framework is obviously V4L- specific, we can't add that to Documentation/SubmittingPatches :-) In any case, I think that the better is to store it at media-build.git tree, and later open such discussions upstream, if we think it is valuable enough. Alternatively, this is something we can document in our wiki. 2) Are there DVB requirements as well for new drivers? We discussed a list of V4L2 requirements in Barcelona, but I wonder if there is a similar list that can be made for DVB drivers. Input on that will be welcome. See below. 3) This document describes the situation we will have when the submaintainers take their place early next year. So please check if I got that part right. 4) In Barcelona we discussed 'tags' for patches to help organize them. I've made a proposal for those in this document. Feedback is very welcome. 5) As discussed in Barcelona there will be git tree maintainers for specific platforms, but we didn't really go into detail who would be responsible for which platform. If you want to maintain a particular platform, then please let me know. 6) The patchwork section is very short at the moment. It should be extended when patchwork gets support to recognize the various tags. 7) Anything else that should be discussed here? Again, remember that this is a rough draft only, so be gentle with me :-) Regards, Hans --- cut here --- General Information === For general information on how to submit patches see: http://linuxtv.org/wiki/index.php/Developer_Section In particular the section 'Submitting Your Work'. This document goes into more detail regarding media specific requirements when submitting patches and what the patch flow looks like in this subsystem. I think we should add a paragraph here saying that rules may have exceptions, when there's a clear reason why a certain submission should need a different criteria. I agree. For instance the uvcvideo driver doesn't use the control framework, and has good reasons not to. This must be the exception rather than the rule, but we might have more than one exception. Yes. Also, IMHO, we should add a notice that this list is not exhaustive, and may be changed, keeping it for at least one or two Kernel cycles, while it doesn't get proofed/matured, as I'm sure we'll forget things. Submitting New Media Drivers When submitting new media drivers
Re: RFC: First draft of guidelines for submitting patches to linux-media
Hi Hans, On Tuesday 11 December 2012 12:50:21 Hans Verkuil wrote: I've added a few quick remarks below. I'll collect all the very useful feedback on Friday and post a new version. After that I'm on vacation for three weeks. On Tue 11 December 2012 11:29:30 Mauro Carvalho Chehab wrote: Em Tue, 11 Dec 2012 00:52:39 +0100 Laurent Pinchart escreveu: On Monday 10 December 2012 16:33:13 Mauro Carvalho Chehab wrote: Em Mon, 10 Dec 2012 14:07:09 +0100 Hans Verkuil escreveu: [snip] Submitting New Media Drivers When submitting new media drivers for inclusion in drivers/staging/media all that is required is that the driver compiles with the latest kernel and that an entry is added to the MAINTAINERS file Please add: and what is missing there for it to be promoted to be a main driver is documented at the TODO file. It should be noticed, however, that it is expected that the driver will be fixed to fulfill the requirements for upstream addition. If a driver at staging lacks relevant patches fixing it for more than a kernel cycle, it can be dropped without further notice. Maybe a single kernel cycle is a bit too strict. The unexpected can happen, so let's not be too harsh. The above is not saying that it should be fixed on one kernel cycle. It says, instead, that drivers without relevant changes during a cycle can be dropped. We'll likely not enforce it too hard, except if we notice some sort of bad faith from the driver's maintainer. And I'm pretty sure we'll always send a notice. The notice will likely be just a patch to the ML c/c the driver's maintainer and the mailing list. As the driver's maintainer email's address might have changed, and/or he might not be subscribed at the ML, it may happen that such email will never reach him. So, the it can be dropped without further notice wording is meant to avoid later complains that from driver's maintainer that he was not warned. It also passes the idea that no ack from the driver's maintainer is needed/expected on such patch. If you think it is badly written, feel free to change it, but keeping this idea. For inclusion as a non-staging driver the requirements are more strict: General requirements: - It must pass checkpatch.pl, but see the note regarding interpreting the output from checkpatch below. - An entry for the driver is added to the MAINTAINERS file. Please add: - Properly use the kernel internal APIs; - Should re-invent the wheel, by adding new defines, math logic, etc that already exists in the Kernel; Should *not* ? :-) Gah... Yeah, it should read, instead: shouldn't. - Errors should be reported as negative numbers, using the Kernel error codes; CodingStyle, chapter 16 (although not as clearly stated). Yes, I know. Yet, this is one of the most frequent bad things we notice on code from new developers. IMHO, it doesn't hurt to explicitly say it here, likely pointing to the CodingStyle corresponding chapter. - typedefs should't be used; CodingStyle, chapter 5. Surprisingly this chapter doesn't mention typedefs for function pointers. Those are very hard to manage without a typedef. Agreed. Same as above: that's the second most frequent bad thing. It seems that windows-way is to create typedefs for each struct and return positive, driver-specific return codes. At least I saw the very same pattern on all windows drivers I looked. V4L2 specific requirements: - Use struct v4l2_device for bridge drivers, use struct v4l2_subdev for sub-device drivers. Please add: - each I2C chip should be mapped as a separate sub-device driver; - Use the control framework for control handling. - Use struct v4l2_fh if the driver supports events (implied by the use of controls) or priority handling. - Use videobuf2 for buffer handling. Mike Krufky will look into extending vb2 to support DVB buffers. Note: using vb2 for VBI devices has not been tested yet, but it should work. Please contact the mailinglist in case of problems with that. - Must pass the v4l2-compliance tests. Please add: - hybrid tuners should be shared with DVB; DVB specific requirements: - Use the DVB core, for both internal and external APIs; - Each I2C-based chip should have its own driver; - Tuners and frontends should be mapped as different drivers; - hybrid tuners should be shared with V4L; Should something be mentioned with regards to DVBv5 and using GET/SET_PROPERTY? [snip] Reviewed-by/Acked-by Within the media subsystem there are three levels of
Re: RFC: First draft of guidelines for submitting patches to linux-media
On Mon 10 December 2012 14:07:09 Hans Verkuil wrote: Hi all, As discussed in Barcelona I would write a text describing requirements for new drivers and what to expect when submitting patches to linux-media. This is a first rough draft and nothing is fixed yet. I have a few open questions: 1) Where to put it? One thing I would propose that we improve is to move the dvb and video4linux directories in Documentation/ to Documentation/media to correctly reflect the drivers/media structure. If we do that, then we can put this document in Documentation/media/SubmittingMediaPatches. Alternatively, this is something we can document in our wiki. 2) Are there DVB requirements as well for new drivers? We discussed a list of V4L2 requirements in Barcelona, but I wonder if there is a similar list that can be made for DVB drivers. Input on that will be welcome. 3) This document describes the situation we will have when the submaintainers take their place early next year. So please check if I got that part right. 4) In Barcelona we discussed 'tags' for patches to help organize them. I've made a proposal for those in this document. Feedback is very welcome. 5) As discussed in Barcelona there will be git tree maintainers for specific platforms, but we didn't really go into detail who would be responsible for which platform. If you want to maintain a particular platform, then please let me know. 6) The patchwork section is very short at the moment. It should be extended when patchwork gets support to recognize the various tags. 7) Anything else that should be discussed here? How to submit patches for a stable kernel. I can never remember, and I'm sure I'm not the only one :-) 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: RFC: First draft of guidelines for submitting patches to linux-media
Em Tue, 11 Dec 2012 12:39:06 +0100 Laurent Pinchart laurent.pinch...@ideasonboard.com escreveu: Hi Mauro, On Tuesday 11 December 2012 08:29:30 Mauro Carvalho Chehab wrote: Em Tue, 11 Dec 2012 00:52:39 +0100 Laurent Pinchart escreveu: On Monday 10 December 2012 16:33:13 Mauro Carvalho Chehab wrote: Em Mon, 10 Dec 2012 14:07:09 +0100 Hans Verkuil escreveu: [snip] Submitting New Media Drivers When submitting new media drivers for inclusion in drivers/staging/media all that is required is that the driver compiles with the latest kernel and that an entry is added to the MAINTAINERS file Please add: and what is missing there for it to be promoted to be a main driver is documented at the TODO file. It should be noticed, however, that it is expected that the driver will be fixed to fulfill the requirements for upstream addition. If a driver at staging lacks relevant patches fixing it for more than a kernel cycle, it can be dropped without further notice. Maybe a single kernel cycle is a bit too strict. The unexpected can happen, so let's not be too harsh. The above is not saying that it should be fixed on one kernel cycle. It says, instead, that drivers without relevant changes during a cycle can be dropped. We'll likely not enforce it too hard, except if we notice some sort of bad faith from the driver's maintainer. That's my point, exactly. The text above just sounds a bit too harsh for my taste, it might even scare people :-) I of course share your point of view that we want developers to understand that they need to work on staging drivers and not let them rot, And I'm pretty sure we'll always send a notice. The notice will likely be just a patch to the ML c/c the driver's maintainer and the mailing list. As the driver's maintainer email's address might have changed, and/or he might not be subscribed at the ML, it may happen that such email will never reach him. So, the it can be dropped without further notice wording is meant to avoid later complains that from driver's maintainer that he was not warned. It also passes the idea that no ack from the driver's maintainer is needed/expected on such patch. If you think it is badly written, feel free to change it, but keeping this idea. What about If no real effort to get a driver out of staging is noticed, the driver can be dropped from the kernel altogether. This policy can be applied over a single kernel release period and without any notice, although we will try our best to communicate with the driver developers and not to enforce the policy too harshly. OK. For inclusion as a non-staging driver the requirements are more strict: General requirements: - It must pass checkpatch.pl, but see the note regarding interpreting the output from checkpatch below. - An entry for the driver is added to the MAINTAINERS file. Please add: - Properly use the kernel internal APIs; - Should re-invent the wheel, by adding new defines, math logic, etc that already exists in the Kernel; Should *not* ? :-) Gah... Yeah, it should read, instead: shouldn't. - Errors should be reported as negative numbers, using the Kernel error codes; CodingStyle, chapter 16 (although not as clearly stated). Yes, I know. Yet, this is one of the most frequent bad things we notice on code from new developers. IMHO, it doesn't hurt to explicitly say it here, likely pointing to the CodingStyle corresponding chapter. Maybe we could just add Follow the CodingStyle guidelines. I prefer to keep those two (errno/typedefs) explicitly, as they're the most common cases. Maybe something like: - Follow Documentation/CodingStyle guidelines. Two common mistakes that should be fixed are to declare typedefs or to return positive, driver-specific error codes on functions, instead of using the standard Linux negative error codes. - typedefs should't be used; CodingStyle, chapter 5. Same as above: that's the second most frequent bad thing. It seems that windows-way is to create typedefs for each struct and return positive, driver-specific return codes. At least I saw the very same pattern on all windows drivers I looked. [snip] Reviewed-by/Acked-by Within the media subsystem there are three levels of maintainership: Mauro Carvalho Chehab is the maintainer of the whole subsystem and the DVB/V4L/IR/Media Controller core code in particular, then there are a number of submaintainers for specific areas of the subsystem: - Kamil Debski: codec (aka memory-to-memory) drivers - Hans de Goede: non-UVC USB webcam drivers - Mike Krufky:
Re: RFC: First draft of guidelines for submitting patches to linux-media
Em Tue, 11 Dec 2012 12:50:21 +0100 Hans Verkuil hverk...@xs4all.nl escreveu: - typedefs should't be used; CodingStyle, chapter 5. Surprisingly this chapter doesn't mention typedefs for function pointers. Those are very hard to manage without a typedef. It seems you detected a potential patch for CodingStyle ;) Anyway, this one of the exceptions that would be accepted. Very unlikely to have it at driver level anyway, as function pointers is more used at the core handlers. DVB specific requirements: - Use the DVB core, for both internal and external APIs; - Each I2C-based chip should have its own driver; - Tuners and frontends should be mapped as different drivers; - hybrid tuners should be shared with V4L; Should something be mentioned with regards to DVBv5 and using GET/SET_PROPERTY? The DVB internal API already enforces it. There's only one thing that could be miss-filled due to DVBv3. So, please add: - dvb_frontend_ops should specify the delivery system, instead of specifying the frontend type via dvb_frontend_ops info.type field. You can also add: - DVB frontends should not implement dummy function handlers; if the function is not implemented, the DVB core should be handled it properly. ... When we point that responsibility to a single person, I'm afraid that the message passed is that he is the sole/main responsible for reviewing core changes, and this is not the case, as it is everybody's responsibility to review v4l/dvb/media controller core changes True, but I think it is a good idea to have a main reviewer assigned whose Acked-by you definitely need. Sure, I can ack a DVB core patch, but that carries a lot less weight than if Mike acks it. I think you'll likely be surprised if you play a little bit with get_maintainer.pl under the DVB tree. For example: $ ./scripts/get_maintainer.pl -f --git-blame drivers/media/dvb-core/dvb_ca_en50221.c Mauro Carvalho Chehab mche...@redhat.com (maintainer:MEDIA INPUT INFRA...,commit_signer:2/2=100%,commits:18/24=75%) Andrew de Quincey adq_...@lidskialf.net (authored lines:41/172=24%,commits:3/24=12%) Matthias Dahl de...@mortal-soul.de (authored lines:21/172=12%) Johannes Stezenbach j...@linuxtv.org (authored lines:19/172=11%,commits:3/24=12%) Harvey Harrison harvey.harri...@gmail.com (authored lines:18/172=10%) Robert Schedel r.sche...@yahoo.de (authored lines:16/172=9%) Andrew Morton a...@osdl.org (commits:7/24=29%) Oliver Endriss o.endr...@gmx.de (commits:6/24=25%) linux-media@vger.kernel.org (open list:MEDIA INPUT INFRA...) linux-ker...@vger.kernel.org (open list) So, your ack for a patch at dvb_ca_en50221.c is as good as Mike's one, as none of you ever touched there, according with git blame. - Guennadi Liakhovetski: soc-camera drivers - Laurent Pinchart: sensor subdev drivers. In addition he'll be the reviewer for Media Controller core patches. I'll change it to a reviewer, as perhaps he won't be able to review everything, and because we're welcoming others to also review it. Ditto. - Hans Verkuil: V4L2 drivers and video A/D and D/A subdev drivers (aka video receivers and transmitters). In addition he'll be the reviewer for V4L2 core patches. I'll change it to a reviewer, as perhaps he won't be able to review everything, and because we're welcoming others to also review it. Ditto. Finally there are maintainers for specific drivers. This is documented in the MAINTAINERS file. When modifying existing code you need to get the Reviewed-by/Acked-by of the maintainer of that code. So CC that maintainer when posting patches. If said maintainer is unavailable then the submaintainer or even Mauro can accept it as well, but that should be the exception, not the rule. Once patches are accepted they will flow through the git tree of the submaintainer to the git tree of the maintainer (Mauro) who will do a final review. There are a few exceptions: code for certain platforms goes through git trees specific to that platform. The submaintainer will still review it and add a acked-by or reviewed-by line, but it will not go through the submaintainer's git tree. The platform maintainers are: TDB - s5p/exynos? - DaVinci? - Omap3? - Omap2? - dvb-usb-v2? Some of those (OMAP2 and OMAP3 at least) are really single drivers. I'm not sure whether we need to list them as platforms. We're currently handling all those Nokia/TI drivers as one platform set of drivers and waiting for Prabhakar to merge them on his tree and submit via git pull request That's only for the davinci code. Prabhakar doesn't handle omap3, that's a single driver at the moment. Ideally there would be an omap directory where TI would maintain the
Re: RFC: First draft of guidelines for submitting patches to linux-media
Em Tue, 11 Dec 2012 13:20:32 +0100 Laurent Pinchart laurent.pinch...@ideasonboard.com escreveu: [snip] - not sure if this is possible: if a v2 patch series is posted, then automatically remove v1. This would require sanity checks: same subject, same author. There's a security issue here as it's easy to spoof a sender e-mail address, but I don't think that we need to care about it. This can be easily detected: if the patch author didn't opt-out, he, and the others who signed the v1 patch will receive a notification when it gets any status change. Also, when the patch gets merged on my tree, the author will receive another email. So, a fake submission can easily be detected. Cheers, 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: RFC: First draft of guidelines for submitting patches to linux-media
Em Tue, 11 Dec 2012 14:15:31 +0100 Hans Verkuil hverk...@xs4all.nl escreveu: On Mon 10 December 2012 14:07:09 Hans Verkuil wrote: Hi all, As discussed in Barcelona I would write a text describing requirements for new drivers and what to expect when submitting patches to linux-media. This is a first rough draft and nothing is fixed yet. I have a few open questions: 1) Where to put it? One thing I would propose that we improve is to move the dvb and video4linux directories in Documentation/ to Documentation/media to correctly reflect the drivers/media structure. If we do that, then we can put this document in Documentation/media/SubmittingMediaPatches. Alternatively, this is something we can document in our wiki. 2) Are there DVB requirements as well for new drivers? We discussed a list of V4L2 requirements in Barcelona, but I wonder if there is a similar list that can be made for DVB drivers. Input on that will be welcome. 3) This document describes the situation we will have when the submaintainers take their place early next year. So please check if I got that part right. 4) In Barcelona we discussed 'tags' for patches to help organize them. I've made a proposal for those in this document. Feedback is very welcome. 5) As discussed in Barcelona there will be git tree maintainers for specific platforms, but we didn't really go into detail who would be responsible for which platform. If you want to maintain a particular platform, then please let me know. 6) The patchwork section is very short at the moment. It should be extended when patchwork gets support to recognize the various tags. 7) Anything else that should be discussed here? How to submit patches for a stable kernel. I can never remember, and I'm sure I'm not the only one :-) The standard way is to add this tag: Cc: sta...@vger.kernel.org eventually with a comment saying to what versions it should be applied, like: Cc: sta...@vger.kernel.org # for v3.5 and upper If it is noticed later the need for it on stable, or a backport is needed, the patch author should send the patch to sta...@vger.kernel.org, c/c the ML, preferably pointing to the upstream commit ID. The patch has to be merged upstream before being merged at stable. -- Cheers, 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: RFC: First draft of guidelines for submitting patches to linux-media
Am 10.12.2012 20:40, schrieb Mauro Carvalho Chehab: [snip] And people beeing subsystem maintainers AND driver maintainers have to find a balance between processing pull requests and reviewing patches. I'm not sure if I have understood yet how this balance should look like... Can you elaborate a bit on this ? At the moment it's ~12 weeks / ~2 weeks. What's the target value ? ;) Please wait for it to be implemented before complaining it ;) The sub-maintainers new schema will start to work likely by Feb/Mar 2013. I don't want to complain (yet ;) ). I'm just trying to understand what is supposed to reduce the review times... Haven't succeeded yet, because the same amount work seems to be redivided among the same amount of maintainer/reviewer resources (=people). Anyway, I will be patient and hope that things will evolve as planed. I will also try to test and/or review patches from other if possible. [snip] So who can get an account / is supposed to access patchwork ? - subsystem maintainers ? - driver maintainers ? - patch creators ? Subsystem maintainers only, except if someone can fix patchwork, adding proper ACL's there to allow patch creators to manage their own patches and sub-system maintainers to delegate work to driver maintainers, without giving them full rights, and being notified about status changes on those driver's patches. Ok, thanks, I think this should be mentioned in the document. Regards, Frank -- 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: RFC: First draft of guidelines for submitting patches to linux-media
Am 10.12.2012 14:07, schrieb Hans Verkuil: snip 3) This document describes the situation we will have when the submaintainers take their place early next year. So please check if I got that part right. ... Reviewed-by/Acked-by Within the media subsystem there are three levels of maintainership: Mauro Carvalho Chehab is the maintainer of the whole subsystem and the DVB/V4L/IR/Media Controller core code in particular, then there are a number of submaintainers for specific areas of the subsystem: - Kamil Debski: codec (aka memory-to-memory) drivers - Hans de Goede: non-UVC USB webcam drivers - Mike Krufky: frontends/tuners/demodulators In addition he'll be the reviewer for DVB core patches. - Guennadi Liakhovetski: soc-camera drivers - Laurent Pinchart: sensor subdev drivers. In addition he'll be the reviewer for Media Controller core patches. - Hans Verkuil: V4L2 drivers and video A/D and D/A subdev drivers (aka video receivers and transmitters). In addition he'll be the reviewer for V4L2 core patches. Finally there are maintainers for specific drivers. This is documented in the MAINTAINERS file. When modifying existing code you need to get the Reviewed-by/Acked-by of the maintainer of that code. So CC that maintainer when posting patches. If said maintainer is unavailable then the submaintainer or even Mauro can accept it as well, but that should be the exception, not the rule. Once patches are accepted they will flow through the git tree of the submaintainer to the git tree of the maintainer (Mauro) who will do a final review. There are a few exceptions: code for certain platforms goes through git trees specific to that platform. The submaintainer will still review it and add a acked-by or reviewed-by line, but it will not go through the submaintainer's git tree. The platform maintainers are: TDB In case patches touch on areas that are the responsibility of multiple submaintainers, then they will decide among one another who will merge the patches. I've read this when the submaintainers take their place early next year, everything will be fine several times now. But can anyone please explain me what's going to change ? AFAICS, the above exactly describes the _current_ situation. We already have sub-maintainers, sub-trees etc, right !? And the people listed above are already doing the same job at the moment. Looking at patchwork, it seems we are behind at least 1 complete kernel release cycle. And the reason seems to be, that (at least some) maintainers don't have the resources to review them in time (no reproaches !). But to me this seems to be no structural problem. If a maintainer (permanently) doesn't have the time to review patches, he should leave maintainership to someone else. So the actual problem seems to be, that we don't have enough maintainers/reviewers, right ? snip Patchwork = Patchwork is an automated system that takes care of all posted patches. It can be found here: http://patchwork.linuxtv.org/project/linux-media/list/ If your patch does not appear in patchwork after [TBD], then check if you used the right patch tags and if your patch is formatted correctly (no HTML, no mangled lines). Whenever you patch changes state you'll get an email informing you about that. What if people send a V2 of a patch (series). Should they mark V1 as superseeded themselves ? And what about maintainers not using patchwork ? Are they nevertheless supposed to take care of the status of their patches ? Regards, Frank -- 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: RFC: First draft of guidelines for submitting patches to linux-media
On Mon December 10 2012 16:56:16 Frank Schäfer wrote: Am 10.12.2012 14:07, schrieb Hans Verkuil: snip 3) This document describes the situation we will have when the submaintainers take their place early next year. So please check if I got that part right. ... Reviewed-by/Acked-by Within the media subsystem there are three levels of maintainership: Mauro Carvalho Chehab is the maintainer of the whole subsystem and the DVB/V4L/IR/Media Controller core code in particular, then there are a number of submaintainers for specific areas of the subsystem: - Kamil Debski: codec (aka memory-to-memory) drivers - Hans de Goede: non-UVC USB webcam drivers - Mike Krufky: frontends/tuners/demodulators In addition he'll be the reviewer for DVB core patches. - Guennadi Liakhovetski: soc-camera drivers - Laurent Pinchart: sensor subdev drivers. In addition he'll be the reviewer for Media Controller core patches. - Hans Verkuil: V4L2 drivers and video A/D and D/A subdev drivers (aka video receivers and transmitters). In addition he'll be the reviewer for V4L2 core patches. Finally there are maintainers for specific drivers. This is documented in the MAINTAINERS file. When modifying existing code you need to get the Reviewed-by/Acked-by of the maintainer of that code. So CC that maintainer when posting patches. If said maintainer is unavailable then the submaintainer or even Mauro can accept it as well, but that should be the exception, not the rule. Once patches are accepted they will flow through the git tree of the submaintainer to the git tree of the maintainer (Mauro) who will do a final review. There are a few exceptions: code for certain platforms goes through git trees specific to that platform. The submaintainer will still review it and add a acked-by or reviewed-by line, but it will not go through the submaintainer's git tree. The platform maintainers are: TDB In case patches touch on areas that are the responsibility of multiple submaintainers, then they will decide among one another who will merge the patches. I've read this when the submaintainers take their place early next year, everything will be fine several times now. I doubt everything will be fine, but I sure hope it will be better at least. In other words, don't expect miracles :-) But can anyone please explain me what's going to change ? AFAICS, the above exactly describes the _current_ situation. We already have sub-maintainers, sub-trees etc, right !? And the people listed above are already doing the same job at the moment. Looking at patchwork, it seems we are behind at least 1 complete kernel release cycle. And the reason seems to be, that (at least some) maintainers don't have the resources to review them in time (no reproaches !). But to me this seems to be no structural problem. If a maintainer (permanently) doesn't have the time to review patches, he should leave maintainership to someone else. So the actual problem seems to be, that we don't have enough maintainers/reviewers, right ? The main problem is that all the work is done by Mauro. Sure, people help out with reviews but a lot of the final administrative and merge effort is done by one person only. In particular the patch flow is something he can't keep up with anymore. So by assigning official submaintainers who get access to patchwork and can process patches quickly we hope that his job will become a lot easier. So the core two changes are 1) giving clear responsibilities to submaintainers and 2) giving submaintainers access to patchwork to keep track of the patches. So patch submitters no longer have to wait until Mauro gets around to cleaning out patchwork. Instead the submaintainers can do that themselves and collect accepted patches in their git tree and post regular pull requests for Mauro. It should simplify things substantially for Mauro, we hope. I think we have enough people doing reviews etc. (although more are always welcome), it's the patchflow itself that is the problem. 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: RFC: First draft of guidelines for submitting patches to linux-media
Em Mon, 10 Dec 2012 17:27:29 +0100 Hans Verkuil hverk...@xs4all.nl escreveu: On Mon December 10 2012 16:56:16 Frank Schäfer wrote: Am 10.12.2012 14:07, schrieb Hans Verkuil: snip 3) This document describes the situation we will have when the submaintainers take their place early next year. So please check if I got that part right. ... Reviewed-by/Acked-by Within the media subsystem there are three levels of maintainership: Mauro Carvalho Chehab is the maintainer of the whole subsystem and the DVB/V4L/IR/Media Controller core code in particular, then there are a number of submaintainers for specific areas of the subsystem: - Kamil Debski: codec (aka memory-to-memory) drivers - Hans de Goede: non-UVC USB webcam drivers - Mike Krufky: frontends/tuners/demodulators In addition he'll be the reviewer for DVB core patches. - Guennadi Liakhovetski: soc-camera drivers - Laurent Pinchart: sensor subdev drivers. In addition he'll be the reviewer for Media Controller core patches. - Hans Verkuil: V4L2 drivers and video A/D and D/A subdev drivers (aka video receivers and transmitters). In addition he'll be the reviewer for V4L2 core patches. Finally there are maintainers for specific drivers. This is documented in the MAINTAINERS file. When modifying existing code you need to get the Reviewed-by/Acked-by of the maintainer of that code. So CC that maintainer when posting patches. If said maintainer is unavailable then the submaintainer or even Mauro can accept it as well, but that should be the exception, not the rule. Once patches are accepted they will flow through the git tree of the submaintainer to the git tree of the maintainer (Mauro) who will do a final review. There are a few exceptions: code for certain platforms goes through git trees specific to that platform. The submaintainer will still review it and add a acked-by or reviewed-by line, but it will not go through the submaintainer's git tree. The platform maintainers are: TDB In case patches touch on areas that are the responsibility of multiple submaintainers, then they will decide among one another who will merge the patches. I've read this when the submaintainers take their place early next year, everything will be fine several times now. I doubt everything will be fine, but I sure hope it will be better at least. In other words, don't expect miracles :-) But can anyone please explain me what's going to change ? AFAICS, the above exactly describes the _current_ situation. We already have sub-maintainers, sub-trees etc, right !? And the people listed above are already doing the same job at the moment. Looking at patchwork, it seems we are behind at least 1 complete kernel release cycle. No, this is not true; git pull requests are typically handled faster, as the material there is submitted either by a driver maintainer or by a sub-maintainer. The delay there for -git is currently 2 weeks, as we closed our merge window at the beginning of -rc7 (expecting that there won't be a -rc8). The very large of individual patches have a longer delays, due to the lack of driver maintainers reviews. And the reason seems to be, that (at least some) maintainers don't have the resources to review them in time (no reproaches !). But to me this seems to be no structural problem. If a maintainer (permanently) doesn't have the time to review patches, he should leave maintainership to someone else. Agreed. So the actual problem seems to be, that we don't have enough maintainers/reviewers, right ? The main problem is that all the work is done by Mauro. Sure, people help out with reviews but a lot of the final administrative and merge effort is done by one person only. In particular the patch flow is something he can't keep up with anymore. So by assigning official submaintainers who get access to patchwork and can process patches quickly we hope that his job will become a lot easier. So the core two changes are 1) giving clear responsibilities to submaintainers and 2) giving submaintainers access to patchwork to keep track of the patches. So patch submitters no longer have to wait until Mauro gets around to cleaning out patchwork. Instead the submaintainers can do that themselves and collect accepted patches in their git tree and post regular pull requests for Mauro. It should simplify things substantially for Mauro, we hope. I think we have enough people doing reviews etc. (although more are always welcome), it's the patchflow itself that is the problem. Yeah, the issue is that both reviewed, non-reviewed and rejected/commented patches go into the very same queue, forcing me to revisit each patch again, even the rejected/commented ones, and the previous versions
Re: RFC: First draft of guidelines for submitting patches to linux-media
On 12/10/2012 07:38 PM, Mauro Carvalho Chehab wrote: Yeah, the issue is that both reviewed, non-reviewed and rejected/commented patches go into the very same queue, forcing me to revisit each patch again, even the rejected/commented ones, and the previous versions of newer patches. By giving rights and responsibilities to the sub-maintainers to manage their stuff directly at patchwork, those patches that tend to stay at patchwork for a long time will likely disappear, and the queue will be cleaner. Is there any change module maintainer responsibility of patch could do what ever he likes to given patch in patchwork? I have looked it already many times but I can drop only my own patches. If someone sends patch to my driver X and I pick it up my GIT tree I would like to mark it superseded for patchwork (which is not possible currently). regards Antti -- http://palosaari.fi/ -- 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: RFC: First draft of guidelines for submitting patches to linux-media
Em Mon, 10 Dec 2012 14:07:09 +0100 Hans Verkuil hverk...@xs4all.nl escreveu: Hi all, As discussed in Barcelona I would write a text describing requirements for new drivers and what to expect when submitting patches to linux-media. This is a first rough draft and nothing is fixed yet. I have a few open questions: 1) Where to put it? Maybe at media-build.git. I'm thinking on putting there, under devel_contrib, the main scripts I use here to handle patches. /me needs some time to sanitize them and add there. One thing I would propose that we improve is to move the dvb and video4linux directories in Documentation/ to Documentation/media to correctly reflect the drivers/media structure. If we do that, then we can put this document in Documentation/media/SubmittingMediaPatches. Hmm... I don't see any other subsystems having their own document for that. We may need to discuss it upstream before doing that, and be prepared to answer why we thing sub-systems would need their own rules there. In any case, I think that the better is to store it at media-build.git tree, and later open such discussions upstream, if we think it is valuable enough. Alternatively, this is something we can document in our wiki. 2) Are there DVB requirements as well for new drivers? We discussed a list of V4L2 requirements in Barcelona, but I wonder if there is a similar list that can be made for DVB drivers. Input on that will be welcome. See below. 3) This document describes the situation we will have when the submaintainers take their place early next year. So please check if I got that part right. 4) In Barcelona we discussed 'tags' for patches to help organize them. I've made a proposal for those in this document. Feedback is very welcome. 5) As discussed in Barcelona there will be git tree maintainers for specific platforms, but we didn't really go into detail who would be responsible for which platform. If you want to maintain a particular platform, then please let me know. 6) The patchwork section is very short at the moment. It should be extended when patchwork gets support to recognize the various tags. 7) Anything else that should be discussed here? Again, remember that this is a rough draft only, so be gentle with me :-) Regards, Hans --- cut here --- General Information === For general information on how to submit patches see: http://linuxtv.org/wiki/index.php/Developer_Section In particular the section 'Submitting Your Work'. This document goes into more detail regarding media specific requirements when submitting patches and what the patch flow looks like in this subsystem. I think we should add a paragraph here saying that rules may have exceptions, when there's a clear reason why a certain submission should need a different criteria. Also, IMHO, we should add a notice that this list is not exhaustive, and may be changed, keeping it for at least one or two Kernel cycles, while it doesn't get proofed/matured, as I'm sure we'll forget things. Submitting New Media Drivers When submitting new media drivers for inclusion in drivers/staging/media all that is required is that the driver compiles with the latest kernel and that an entry is added to the MAINTAINERS file Please add: and what is missing there for it to be promoted to be a main driver is documented at the TODO file. It should be noticed, however, that it is expected that the driver will be fixed to fulfill the requirements for upstream addition. If a driver at staging lacks relevant patches fixing it for more than a kernel cycle, it can be dropped without further notice. For inclusion as a non-staging driver the requirements are more strict: General requirements: - It must pass checkpatch.pl, but see the note regarding interpreting the output from checkpatch below. - An entry for the driver is added to the MAINTAINERS file. Please add: - Properly use the kernel internal APIs; - Should re-invent the wheel, by adding new defines, math logic, etc that already exists in the Kernel; - Errors should be reported as negative numbers, using the Kernel error codes; - typedefs should't be used; V4L2 specific requirements: - Use struct v4l2_device for bridge drivers, use struct v4l2_subdev for sub-device drivers. Please add: - each I2C chip should be mapped as a separate sub-device driver; - Use the control framework for control handling. - Use struct v4l2_fh if the driver supports events (implied by the use of controls) or priority handling. - Use videobuf2 for buffer handling. Mike Krufky will look into extending vb2 to support DVB buffers. Note: using vb2 for VBI devices has not been tested yet, but it should work. Please contact the mailinglist in case of problems with that. - Must pass the v4l2-compliance
Re: RFC: First draft of guidelines for submitting patches to linux-media
Em Mon, 10 Dec 2012 19:45:40 +0200 Antti Palosaari cr...@iki.fi escreveu: On 12/10/2012 07:38 PM, Mauro Carvalho Chehab wrote: Yeah, the issue is that both reviewed, non-reviewed and rejected/commented patches go into the very same queue, forcing me to revisit each patch again, even the rejected/commented ones, and the previous versions of newer patches. By giving rights and responsibilities to the sub-maintainers to manage their stuff directly at patchwork, those patches that tend to stay at patchwork for a long time will likely disappear, and the queue will be cleaner. Is there any change module maintainer responsibility of patch could do what ever he likes to given patch in patchwork? I have looked it already many times but I can drop only my own patches. If someone sends patch to my driver X and I pick it up my GIT tree I would like to mark it superseded for patchwork (which is not possible currently). Patchwork's ACL is very limited. It has 3 types there: - People (every email it detects); - User (the ones that created a password); - Project maintainers; A people can't do anything special, except be promoted to users, by setting a password for him. An user can only set his emails, enable/disable opt-out/opt-in, set his primary project and the number of patches per page. The Project maintainers can do everything in the project. It would be great to have a feature there allowing the user to change the status of their own patches, and to let the project maintainers to delegate a patch to an user[1]. [1] well, I think it can delegate it right now, but only a project maintainer can change the patch status, so, delegation doesn't work if the delegated user is not a project owner. 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: RFC: First draft of guidelines for submitting patches to linux-media
Am 10.12.2012 18:38, schrieb Mauro Carvalho Chehab: Em Mon, 10 Dec 2012 17:27:29 +0100 Hans Verkuil hverk...@xs4all.nl escreveu: On Mon December 10 2012 16:56:16 Frank Schäfer wrote: Am 10.12.2012 14:07, schrieb Hans Verkuil: snip 3) This document describes the situation we will have when the submaintainers take their place early next year. So please check if I got that part right. ... Reviewed-by/Acked-by Within the media subsystem there are three levels of maintainership: Mauro Carvalho Chehab is the maintainer of the whole subsystem and the DVB/V4L/IR/Media Controller core code in particular, then there are a number of submaintainers for specific areas of the subsystem: - Kamil Debski: codec (aka memory-to-memory) drivers - Hans de Goede: non-UVC USB webcam drivers - Mike Krufky: frontends/tuners/demodulators In addition he'll be the reviewer for DVB core patches. - Guennadi Liakhovetski: soc-camera drivers - Laurent Pinchart: sensor subdev drivers. In addition he'll be the reviewer for Media Controller core patches. - Hans Verkuil: V4L2 drivers and video A/D and D/A subdev drivers (aka video receivers and transmitters). In addition he'll be the reviewer for V4L2 core patches. Finally there are maintainers for specific drivers. This is documented in the MAINTAINERS file. When modifying existing code you need to get the Reviewed-by/Acked-by of the maintainer of that code. So CC that maintainer when posting patches. If said maintainer is unavailable then the submaintainer or even Mauro can accept it as well, but that should be the exception, not the rule. Once patches are accepted they will flow through the git tree of the submaintainer to the git tree of the maintainer (Mauro) who will do a final review. There are a few exceptions: code for certain platforms goes through git trees specific to that platform. The submaintainer will still review it and add a acked-by or reviewed-by line, but it will not go through the submaintainer's git tree. The platform maintainers are: TDB In case patches touch on areas that are the responsibility of multiple submaintainers, then they will decide among one another who will merge the patches. I've read this when the submaintainers take their place early next year, everything will be fine several times now. I doubt everything will be fine, but I sure hope it will be better at least. In other words, don't expect miracles :-) But can anyone please explain me what's going to change ? AFAICS, the above exactly describes the _current_ situation. We already have sub-maintainers, sub-trees etc, right !? And the people listed above are already doing the same job at the moment. Looking at patchwork, it seems we are behind at least 1 complete kernel release cycle. No, this is not true; git pull requests are typically handled faster, as the material there is submitted either by a driver maintainer or by a sub-maintainer. The delay there for -git is currently 2 weeks, as we closed our merge window at the beginning of -rc7 (expecting that there won't be a -rc8). But it's not git pull request vs. patches sent to the list directly, it's reviewed vs. not reviewed, right ? The very large of individual patches have a longer delays, due to the lack of driver maintainers reviews. That's what I said, the problem is that we lack maintainers/reviewers. And people beeing subsystem maintainers AND driver maintainers have to find a balance between processing pull requests and reviewing patches. I'm not sure if I have understood yet how this balance should look like... Can you elaborate a bit on this ? At the moment it's ~12 weeks / ~2 weeks. What's the target value ? ;) And the reason seems to be, that (at least some) maintainers don't have the resources to review them in time (no reproaches !). But to me this seems to be no structural problem. If a maintainer (permanently) doesn't have the time to review patches, he should leave maintainership to someone else. Agreed. So the actual problem seems to be, that we don't have enough maintainers/reviewers, right ? The main problem is that all the work is done by Mauro. Sure, people help out with reviews but a lot of the final administrative and merge effort is done by one person only. In particular the patch flow is something he can't keep up with anymore. So by assigning official submaintainers who get access to patchwork and can process patches quickly we hope that his job will become a lot easier. So the core two changes are 1) giving clear responsibilities to submaintainers and 2) giving submaintainers access to patchwork to keep track of the patches. So patch submitters no longer have to wait until Mauro gets around to cleaning out patchwork. Instead the submaintainers can do that themselves and collect accepted patches in their git tree and post regular pull requests for Mauro. It
Re: RFC: First draft of guidelines for submitting patches to linux-media
Em Mon, 10 Dec 2012 20:17:23 +0100 Frank Schäfer fschaefer@googlemail.com escreveu: Am 10.12.2012 18:38, schrieb Mauro Carvalho Chehab: Em Mon, 10 Dec 2012 17:27:29 +0100 Hans Verkuil hverk...@xs4all.nl escreveu: On Mon December 10 2012 16:56:16 Frank Schäfer wrote: Am 10.12.2012 14:07, schrieb Hans Verkuil: snip 3) This document describes the situation we will have when the submaintainers take their place early next year. So please check if I got that part right. ... Reviewed-by/Acked-by Within the media subsystem there are three levels of maintainership: Mauro Carvalho Chehab is the maintainer of the whole subsystem and the DVB/V4L/IR/Media Controller core code in particular, then there are a number of submaintainers for specific areas of the subsystem: - Kamil Debski: codec (aka memory-to-memory) drivers - Hans de Goede: non-UVC USB webcam drivers - Mike Krufky: frontends/tuners/demodulators In addition he'll be the reviewer for DVB core patches. - Guennadi Liakhovetski: soc-camera drivers - Laurent Pinchart: sensor subdev drivers. In addition he'll be the reviewer for Media Controller core patches. - Hans Verkuil: V4L2 drivers and video A/D and D/A subdev drivers (aka video receivers and transmitters). In addition he'll be the reviewer for V4L2 core patches. Finally there are maintainers for specific drivers. This is documented in the MAINTAINERS file. When modifying existing code you need to get the Reviewed-by/Acked-by of the maintainer of that code. So CC that maintainer when posting patches. If said maintainer is unavailable then the submaintainer or even Mauro can accept it as well, but that should be the exception, not the rule. Once patches are accepted they will flow through the git tree of the submaintainer to the git tree of the maintainer (Mauro) who will do a final review. There are a few exceptions: code for certain platforms goes through git trees specific to that platform. The submaintainer will still review it and add a acked-by or reviewed-by line, but it will not go through the submaintainer's git tree. The platform maintainers are: TDB In case patches touch on areas that are the responsibility of multiple submaintainers, then they will decide among one another who will merge the patches. I've read this when the submaintainers take their place early next year, everything will be fine several times now. I doubt everything will be fine, but I sure hope it will be better at least. In other words, don't expect miracles :-) But can anyone please explain me what's going to change ? AFAICS, the above exactly describes the _current_ situation. We already have sub-maintainers, sub-trees etc, right !? And the people listed above are already doing the same job at the moment. Looking at patchwork, it seems we are behind at least 1 complete kernel release cycle. No, this is not true; git pull requests are typically handled faster, as the material there is submitted either by a driver maintainer or by a sub-maintainer. The delay there for -git is currently 2 weeks, as we closed our merge window at the beginning of -rc7 (expecting that there won't be a -rc8). But it's not git pull request vs. patches sent to the list directly, it's reviewed vs. not reviewed, right ? A patch reviewed by a sub-maintainer/driver maintainers typically goes to me via a git pull request. The very large of individual patches have a longer delays, due to the lack of driver maintainers reviews. That's what I said, the problem is that we lack maintainers/reviewers. We (used to) lack to have sub-maintainers formally (except for gspca and soc_camera). Even driver's maintainership is currently shady, as most drivers lack a MAINTAINERS entry. That is in the process of being fixed. And people beeing subsystem maintainers AND driver maintainers have to find a balance between processing pull requests and reviewing patches. I'm not sure if I have understood yet how this balance should look like... Can you elaborate a bit on this ? At the moment it's ~12 weeks / ~2 weeks. What's the target value ? ;) Please wait for it to be implemented before complaining it ;) The sub-maintainers new schema will start to work likely by Feb/Mar 2013. Also, the reviewing process is not equal to all patches: trivial patches can be merged quicker; core API changes should take a longer time, as it is expected to have more review before merging them. And the reason seems to be, that (at least some) maintainers don't have the resources to review them in time (no reproaches !). But to me this seems to be no structural problem. If a maintainer (permanently) doesn't have the time to review patches, he should leave maintainership to someone else. Agreed. So the actual problem seems
Re: RFC: First draft of guidelines for submitting patches to linux-media
Hi, On Monday 10 December 2012 16:33:13 Mauro Carvalho Chehab wrote: Em Mon, 10 Dec 2012 14:07:09 +0100 Hans Verkuil escreveu: Hi all, As discussed in Barcelona I would write a text describing requirements for new drivers and what to expect when submitting patches to linux-media. This is a first rough draft and nothing is fixed yet. I have a few open questions: 1) Where to put it? Maybe at media-build.git. Not everybody uses (or is even aware of) media-build. The goal here is to make sure that new driver developers will run into the guidelines before then spend months writing code that can't be upstreamed. Documentation/ thus looks like a good place to me. It might be a good idea to add a reference to the guidelines in the API DocBook documentation, regardless of where the guidelines end up being stored. If a developer reads a single document only it will probably be the API reference. I'm thinking on putting there, under devel_contrib, the main scripts I use here to handle patches. /me needs some time to sanitize them and add there. One thing I would propose that we improve is to move the dvb and video4linux directories in Documentation/ to Documentation/media to correctly reflect the drivers/media structure. If we do that, then we can put this document in Documentation/media/SubmittingMediaPatches. Hmm... I don't see any other subsystems having their own document for that. We may need to discuss it upstream before doing that, and be prepared to answer why we thing sub-systems would need their own rules there. Things like requiring the use of the control framework is obviously V4L- specific, we can't add that to Documentation/SubmittingPatches :-) In any case, I think that the better is to store it at media-build.git tree, and later open such discussions upstream, if we think it is valuable enough. Alternatively, this is something we can document in our wiki. 2) Are there DVB requirements as well for new drivers? We discussed a list of V4L2 requirements in Barcelona, but I wonder if there is a similar list that can be made for DVB drivers. Input on that will be welcome. See below. 3) This document describes the situation we will have when the submaintainers take their place early next year. So please check if I got that part right. 4) In Barcelona we discussed 'tags' for patches to help organize them. I've made a proposal for those in this document. Feedback is very welcome. 5) As discussed in Barcelona there will be git tree maintainers for specific platforms, but we didn't really go into detail who would be responsible for which platform. If you want to maintain a particular platform, then please let me know. 6) The patchwork section is very short at the moment. It should be extended when patchwork gets support to recognize the various tags. 7) Anything else that should be discussed here? Again, remember that this is a rough draft only, so be gentle with me :-) Regards, Hans --- cut here --- General Information === For general information on how to submit patches see: http://linuxtv.org/wiki/index.php/Developer_Section In particular the section 'Submitting Your Work'. This document goes into more detail regarding media specific requirements when submitting patches and what the patch flow looks like in this subsystem. I think we should add a paragraph here saying that rules may have exceptions, when there's a clear reason why a certain submission should need a different criteria. I agree. For instance the uvcvideo driver doesn't use the control framework, and has good reasons not to. This must be the exception rather than the rule, but we might have more than one exception. Also, IMHO, we should add a notice that this list is not exhaustive, and may be changed, keeping it for at least one or two Kernel cycles, while it doesn't get proofed/matured, as I'm sure we'll forget things. Submitting New Media Drivers When submitting new media drivers for inclusion in drivers/staging/media all that is required is that the driver compiles with the latest kernel and that an entry is added to the MAINTAINERS file Please add: and what is missing there for it to be promoted to be a main driver is documented at the TODO file. It should be noticed, however, that it is expected that the driver will be fixed to fulfill the requirements for upstream addition. If a driver at staging lacks relevant patches fixing it for more than a kernel cycle, it can be dropped without further notice. Maybe a single kernel cycle is a bit too strict. The unexpected can happen, so let's not be too harsh. And I'm pretty sure we'll always send a notice. For inclusion as a non-staging driver the requirements are more strict:
