Development: How to submit patches: Difference between revisions
(→Patch Preparation: point to linux-media repo, plus some small copy-edits) |
|||
(25 intermediate revisions by 6 users not shown) | |||
Line 1: | Line 1: | ||
This article provides an overview for submitting patches against the [[What is V4L or DVB?|V4L-DVB source code]] (for either new or existing kernel driver modules and/or documentation), and as well as for the [[LinuxTV dvb-apps|dvb-apps]] source. For general references in regards as to how to develop support for a particular device or in writing a new device driver, see [[Development: How to add support for a device|here]]. |
This article provides an overview for submitting patches against the [[What is V4L or DVB?|V4L-DVB source code]] (for either new or existing kernel driver modules and/or documentation), and as well as for the [[LinuxTV dvb-apps|dvb-apps]] source. For general references in regards as to how to develop support for a particular device or in writing a new device driver, see [[Development: How to add support for a device|here]]. |
||
Initial dvb scan tables/files can also be [[Scantables|submitted]]. |
|||
== Patch Preparation == |
== Patch Preparation == |
||
For |
For Kernel media patches, they should be created against the [http://git.linuxtv.org/media_tree.git Linux Kernel master linux_media git tree]; for instructions on obtaining and building these sources, see the "[[How to Obtain, Build and Install V4L-DVB Device Drivers]]" article. |
||
Patches for the backport tree should be made against the [http://git.linuxtv.org/media_build.git media_build] tree. Please notice that this tree contains only the building system for backports. The actual drivers are at the [http://git.linuxtv.org/media_tree.git Linux Kernel master linux_media git tree]; |
|||
For patches for the userspace applications hosted at [http://linuxtv.org LinuxTV], the patch should be generated against one of the corresponding upstream trees: |
|||
* v4l-utils/libv4l/libdvbv5: [http://git.linuxtv.org/v4l-utils.git v4l-utils ''git'' tree] |
|||
* DTV scan tables: [http://git.linuxtv.org/dtv-scan-tables.git dtv-scan-tables ''git'' tree] |
|||
* tvtime: [http://git.linuxtv.org/tvtime.git TVtime ''git'' tree] |
|||
* xawtv3: [http://git.linuxtv.org/xawtv3.git XawTV version 3 ''git'' tree] |
|||
* xawtv4: [http://git.linuxtv.org/xawtv4.git XawTV version 4 ''git'' tree] |
|||
* dvb-apps: [http://linuxtv.org/hg/dvb-apps dvb-apps ''mercurial'' tree] |
|||
The patch should contain an unified diff between the latest code at the tree and the code you modified. The best way to do it is by running the command ''git diff'' (or ''hg diff'' for a ''mercurial'' tree]. They should also be compatible with the Kernel licenses. See [[Development:_How_to_submit_patches#License|License section]] for more details. |
|||
Before submitting your Kernel patch, please run ''make checkpatch''. This will tell the build system to run a script that checks for coding style violations. |
|||
Post your patches to the [mailto:majordomo@vger.kernel.org?body=subscribe%20linux-media Linux-Media Mailing List] for review and testing. [http://vger.kernel.org/vger-lists.html#linux-media Subscription to the mailing list] is recommended, though it is not required. |
Post your patches to the [mailto:majordomo@vger.kernel.org?body=subscribe%20linux-media Linux-Media Mailing List] for review and testing. [http://vger.kernel.org/vger-lists.html#linux-media Subscription to the mailing list] is recommended, though it is not required. |
||
Line 14: | Line 29: | ||
:* Explain what the patch does and to what hardware it applies. Note: All comments you add on your patch will be part of the commit message (except for the meta-tags) |
:* Explain what the patch does and to what hardware it applies. Note: All comments you add on your patch will be part of the commit message (except for the meta-tags) |
||
:* Document your work where appropriate (i.e. in the form of patches to the Documentation/video4linux or Documentation/dvb files) |
:* Document your work where appropriate (i.e. in the form of patches to the Documentation/video4linux or Documentation/dvb files) |
||
:* Add a '''Signed-off-by: Your name |
:* Add a '''Signed-off-by: Your name <your@email>''' as a [[Development: Submitting Patches#Developer.27s_Certificate_of_Origin_1.1|Developer's Certificate of Origin 1.1 ]] |
||
:* Send the patch inline, not as an attachment (unless otherwise asked to do so) ... patches presented in the form of inline text in the body of an email are easier to deal with from the perspective of a peer review process (for more information, see the |
:* Send the patch inline, not as an attachment (unless otherwise asked to do so) ... patches presented in the form of inline text in the body of an email are easier to deal with from the perspective of a peer review process (for more information, see the '''/usr/src/linux/Documentation/email-clients.txt''' file; a current copy of which can also be found online [http://lxr.linux.no/linux+v2.6.28.5/Documentation/email-clients.txt here]). |
||
:* '''Note''': various web mail interfaces seem to be problematic for patch submission, in that: |
:* '''Note''': various web mail interfaces seem to be problematic for patch submission, in that: |
||
:** they may break the patch (e.g. line wrapping it) or |
:** they may break the patch (e.g. line wrapping it) or |
||
Line 21: | Line 36: | ||
Hint: There is also a [[Development: Linux Kernel patch submittal checklist|checklist for patch submission]] |
Hint: There is also a [[Development: Linux Kernel patch submittal checklist|checklist for patch submission]] |
||
== License == |
|||
All files added to the Linux Kernel should be GPL 2.0 compatible. It can be licenced to just GPL 2.0 or have a broader license, like GPL 2.0 or later, or being dual-licensed (like GPL 2.0 or BSD). |
|||
We're now using [[https://www.kernel.org/doc/html/latest/process/license-rules.html#kernel-licensing SPDX headers]] for tagging the license of the Kernel files. |
|||
For C files, that is done by simply adding, at the first line of a file, the following tag: |
|||
// SPDX-License-Identifier: GPL-2.0-only |
|||
For header files, this is done by adding, instead: |
|||
/* SPDX-License-Identifier: GPL-2.0-only */ |
|||
=== License on documentation files === |
|||
In the case of documentation files, due to historical reasons, our uAPI files were licensed using GNU Free Document License with no invariant sections. For new files, we're now ensuring that all uAPI documentation files will be dual licensed with GFDL or GPL 2.0. |
|||
Unfortunately, [[https://github.com/spdx/license-list-XML/issues/686 currently]], there's no way to indicate it via SPDX. So, the rules are: |
|||
1. All new files under Documentation/media/uapi should be dual licensed GFDL or GPL. |
|||
So, for files using the ReStructured Text (rst) format, they should have the following lines at the beginning: |
|||
.. This file is dual-licensed: you can use it either under the terms |
|||
.. of the GPL or the GFDL 1.1+ license, at your option. Note that this |
|||
.. dual licensing only applies to this file, and not this project as a |
|||
.. whole. |
|||
.. |
|||
.. a) This file is free software; you can redistribute it and/or |
|||
.. modify it under the terms of the GNU General Public License as |
|||
.. published by the Free Software Foundation version 2 of |
|||
.. the License. |
|||
.. |
|||
.. This file is distributed in the hope that it will be useful, |
|||
.. but WITHOUT ANY WARRANTY; without even the implied warranty of |
|||
.. MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
|||
.. GNU General Public License for more details. |
|||
.. |
|||
.. Or, alternatively, |
|||
.. |
|||
.. b) Permission is granted to copy, distribute and/or modify this |
|||
.. document under the terms of the GNU Free Documentation License, |
|||
.. Version 1.1 or any later version published by the Free Software |
|||
.. Foundation, with no Invariant Sections, no Front-Cover Texts |
|||
.. and no Back-Cover Texts. A copy of the license is included at |
|||
.. Documentation/media/uapi/fdl-appendix.rst. |
|||
.. |
|||
.. TODO: replace it to GPL-2.0 OR GFDL-1.1-or-later WITH no-invariant-sections |
|||
2. All other documentation files should be GPL-2.0 compatible. So, if the file is a ReStructured Text file licenced with just GPL-2.0, they should have: |
|||
.. SPDX-License-Identifier: GPL-2.0 |
|||
== Example of a good patch submission email == |
|||
The following example (based on a real patch submission) shows, in practice, how |
|||
a good patch should look like: |
|||
From: Patch Developer |
|||
Date: Thu, 27 Sep 2012 05:32:52 -0300 |
|||
Subject: [PATCH] davinci: vpif: capture/display: fix race condition |
|||
channel_first_int[][] variable is used as a flag for the ISR, |
|||
This flag was being set after enabling the interrupts, There |
|||
where situations when the isr occurred even before the flag was set |
|||
due to which it was causing the application hang. |
|||
This patch sets channel_first_int[][] flag just before enabling the |
|||
interrupt. |
|||
Reported-by: Some User |
|||
Signed-off-by: Patch Developer |
|||
Reviewed-by: Patch Reviewer |
|||
--- |
|||
v2: Coding Style fixed, as per-review. |
|||
drivers/media/platform/davinci/vpif_capture.c | 1 + |
|||
1 file changed, 1 insertion(+) |
|||
diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c |
|||
index 13aa46d..0bafeca 100644 |
|||
--- a/drivers/media/platform/davinci/vpif_capture.c |
|||
+++ b/drivers/media/platform/davinci/vpif_capture.c |
|||
@@ -339,6 +339,7 @@ static int vpif_start_streaming(struct vb2_queue *vq, unsigned int count) |
|||
* Set interrupt for both the fields in VPIF Register enable channel in |
|||
* VPIF register |
|||
*/ |
|||
+ channel_first_int[VPIF_VIDEO_INDEX][ch->channel_id] = 1; |
|||
if ((VPIF_CHANNEL0_VIDEO == ch->channel_id)) { |
|||
channel0_intr_assert(); |
|||
channel0_intr_enable(1); |
|||
*The fist part contains the email header and the first part of the email body, with the patch description, followed by Signed-off/Reviewed-by/Acked-By tags. |
|||
Note: For patches submitted by one person, but authored by another one, the '''first line''' of the email body should contain a ''From:'' tag with the name and email of the original author. For example, if the above e-mail was sent by ''Patch Reviewer'', it would contain, instead: |
|||
From: Patch Reviewer |
|||
Date: Thu, 27 Sep 2012 05:32:52 -0300 |
|||
Subject: [PATCH] davinci: vpif: capture/display: fix race condition |
|||
From: Patch Developer |
|||
channel_first_int[][] variable is used as a flag for the ISR, |
|||
This flag was being set after enabling the interrupts, There |
|||
where situations when the isr occurred even before the flag was set |
|||
due to which it was causing the application hang. |
|||
This patch sets channel_first_int[][] flag just before enabling the |
|||
interrupt. |
|||
Reported-by: Some User |
|||
Signed-off-by: Patch Developer |
|||
Signed-off-by: Patch Reviewer |
|||
*The second part is optional, separated by "---", contains the patch diffstat. It can also contain any notice for the patch reviewers to read. The second part won't be merged at the patch commit. |
|||
*The third part is the diff, using unified diff, -p1 format. It is typically generated by a "git diff" command. |
|||
Note: on the above, ''Some User'', ''Patch Developer'' and ''Patch Reviewer'' should be the full email of real persons, in the form: ''some name <some email>''. |
|||
== Firmware submission == |
== Firmware submission == |
||
Line 40: | Line 176: | ||
== After you've Submitted: What happens Next? == |
== After you've Submitted: What happens Next? == |
||
In conjunction with the move to the new Linux-media mailing list, V4L-DVB is now using the [http://patchwork. |
In conjunction with the move to the new Linux-media mailing list, V4L-DVB is now using the [http://patchwork.linuxtv.org/project/linux-media/list/ Patchwork tool] for aggregating patches sent into the list (you can read more about it [http://www.linuxtv.org/news.php?entry=2009-01-06.mchehab here] and [http://www.linuxtv.org/news.php?entry=2011-09-18.mchehab here]). In the past, it was, unfortunately, not uncommon for patches to be overlooked and become lost on the V4L-DVB mailing lists; thus making the adoption of the patchwork tool a very welcome addition. So, provided you have correctly submitted your patch (as discussed above), the steps towards having your code adopted will have automatically been put into motion. You can, of course, check to see the status of your submission from the [http://patchwork.kernel.org/project/linux-media/list/ Patchwork webpage]. If patchwork has not picked up your patch (after a reasonable period of time has elapsed), it is quite probable that your submission was incorrect for some reason; please review the information contained in the above section and try again. |
||
<div style="border: solid 1px; border-color: blue; margin: 1em; padding: 0.5em; background-color: Lavender;">"The most difficult problem isn't fixing bugs, but fixing bugs without breaking other configurations. There are many: different cards, different TV norms, whereas most of the developers can test only one TV norm." - Gerd Knorr</div> |
|||
After being picked up by patchwork, the first thing you should expect next is that a [[Development: Code Review|code review]] will be performed, and often this is by various parties. This may lead to a series of requests for you to fix any observed problems and require you to then resubmit your work, by repeating the same steps outlined above, until everyone is happy with the submission. In other words: wash, rinse, repeat ;) |
After being picked up by patchwork, the first thing you should expect next is that a [[Development: Code Review|code review]] will be performed, and often this is by various parties. This may lead to a series of requests for you to fix any observed problems and require you to then resubmit your work, by repeating the same steps outlined above, until everyone is happy with the submission. In other words: wash, rinse, repeat ;) |
||
Then, when your patch is accepted, it will initially be applied to the main |
Then, when your patch is accepted, it will initially be applied to the main linux-media git tree. Once tested and integrated, such patches are later merged into a git tree by the V4L-DVB maintainer and, upon request, periodically pulled by Linus into one of his own git trees in an intermediate step towards final inclusion into the Linux kernel. |
||
[[Category:Development]] |
[[Category:Development]] |
Latest revision as of 15:38, 17 December 2018
This article provides an overview for submitting patches against the V4L-DVB source code (for either new or existing kernel driver modules and/or documentation), and as well as for the dvb-apps source. For general references in regards as to how to develop support for a particular device or in writing a new device driver, see here.
Initial dvb scan tables/files can also be submitted.
Patch Preparation
For Kernel media patches, they should be created against the Linux Kernel master linux_media git tree; for instructions on obtaining and building these sources, see the "How to Obtain, Build and Install V4L-DVB Device Drivers" article.
Patches for the backport tree should be made against the media_build tree. Please notice that this tree contains only the building system for backports. The actual drivers are at the Linux Kernel master linux_media git tree;
For patches for the userspace applications hosted at LinuxTV, the patch should be generated against one of the corresponding upstream trees:
- v4l-utils/libv4l/libdvbv5: v4l-utils git tree
- DTV scan tables: dtv-scan-tables git tree
- tvtime: TVtime git tree
- xawtv3: XawTV version 3 git tree
- xawtv4: XawTV version 4 git tree
- dvb-apps: dvb-apps mercurial tree
The patch should contain an unified diff between the latest code at the tree and the code you modified. The best way to do it is by running the command git diff (or hg diff for a mercurial tree]. They should also be compatible with the Kernel licenses. See License section for more details.
Before submitting your Kernel patch, please run make checkpatch. This will tell the build system to run a script that checks for coding style violations.
Post your patches to the Linux-Media Mailing List for review and testing. Subscription to the mailing list is recommended, though it is not required.
Follow the guidelines in Submitting Patches (cf. jgarzik's version), including:
- Verify best-practice kernel coding style
- Use [PATCH] in the subject line to get attention ... Note: the patchwork tool (discussed below) actually does NOT rely upon this label for detection of patches; rather, patchwork utilizes logic algorithms to detect for the presence of a patch within an email message. That being the case, the "[PATCH]" flag serves only to alert your human counterparts/peers on the mailing list of your submission
- Explain what the patch does and to what hardware it applies. Note: All comments you add on your patch will be part of the commit message (except for the meta-tags)
- Document your work where appropriate (i.e. in the form of patches to the Documentation/video4linux or Documentation/dvb files)
- Add a Signed-off-by: Your name <your@email> as a Developer's Certificate of Origin 1.1
- Send the patch inline, not as an attachment (unless otherwise asked to do so) ... patches presented in the form of inline text in the body of an email are easier to deal with from the perspective of a peer review process (for more information, see the /usr/src/linux/Documentation/email-clients.txt file; a current copy of which can also be found online here).
- Note: various web mail interfaces seem to be problematic for patch submission, in that:
- they may break the patch (e.g. line wrapping it) or
- in the case where you have sent the patch as an attachment, the emailer may use the wrong mime encoding type ... (web mailers often wrongly use "application/octet-stream" for diffs, whereas the proper type is "text/x-patch" ... Note: the patchwork tool (discussed below) is robust in that it supports both mime types "text/x-patch" and "text/plain", but if the emailer has sent it with a different type, the attachment will be disregarded/discarded.
Hint: There is also a checklist for patch submission
License
All files added to the Linux Kernel should be GPL 2.0 compatible. It can be licenced to just GPL 2.0 or have a broader license, like GPL 2.0 or later, or being dual-licensed (like GPL 2.0 or BSD).
We're now using [SPDX headers] for tagging the license of the Kernel files.
For C files, that is done by simply adding, at the first line of a file, the following tag:
// SPDX-License-Identifier: GPL-2.0-only
For header files, this is done by adding, instead:
/* SPDX-License-Identifier: GPL-2.0-only */
License on documentation files
In the case of documentation files, due to historical reasons, our uAPI files were licensed using GNU Free Document License with no invariant sections. For new files, we're now ensuring that all uAPI documentation files will be dual licensed with GFDL or GPL 2.0.
Unfortunately, [currently], there's no way to indicate it via SPDX. So, the rules are:
1. All new files under Documentation/media/uapi should be dual licensed GFDL or GPL.
So, for files using the ReStructured Text (rst) format, they should have the following lines at the beginning:
.. This file is dual-licensed: you can use it either under the terms .. of the GPL or the GFDL 1.1+ license, at your option. Note that this .. dual licensing only applies to this file, and not this project as a .. whole. .. .. a) This file is free software; you can redistribute it and/or .. modify it under the terms of the GNU General Public License as .. published by the Free Software Foundation version 2 of .. the License. .. .. This file is distributed in the hope that it will be useful, .. but WITHOUT ANY WARRANTY; without even the implied warranty of .. MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the .. GNU General Public License for more details. .. .. Or, alternatively, .. .. b) Permission is granted to copy, distribute and/or modify this .. document under the terms of the GNU Free Documentation License, .. Version 1.1 or any later version published by the Free Software .. Foundation, with no Invariant Sections, no Front-Cover Texts .. and no Back-Cover Texts. A copy of the license is included at .. Documentation/media/uapi/fdl-appendix.rst. .. .. TODO: replace it to GPL-2.0 OR GFDL-1.1-or-later WITH no-invariant-sections
2. All other documentation files should be GPL-2.0 compatible. So, if the file is a ReStructured Text file licenced with just GPL-2.0, they should have:
.. SPDX-License-Identifier: GPL-2.0
Example of a good patch submission email
The following example (based on a real patch submission) shows, in practice, how a good patch should look like:
From: Patch Developer Date: Thu, 27 Sep 2012 05:32:52 -0300 Subject: [PATCH] davinci: vpif: capture/display: fix race condition channel_first_int[][] variable is used as a flag for the ISR, This flag was being set after enabling the interrupts, There where situations when the isr occurred even before the flag was set due to which it was causing the application hang. This patch sets channel_first_int[][] flag just before enabling the interrupt. Reported-by: Some User Signed-off-by: Patch Developer Reviewed-by: Patch Reviewer
--- v2: Coding Style fixed, as per-review. drivers/media/platform/davinci/vpif_capture.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c index 13aa46d..0bafeca 100644 --- a/drivers/media/platform/davinci/vpif_capture.c +++ b/drivers/media/platform/davinci/vpif_capture.c @@ -339,6 +339,7 @@ static int vpif_start_streaming(struct vb2_queue *vq, unsigned int count) * Set interrupt for both the fields in VPIF Register enable channel in * VPIF register */ + channel_first_int[VPIF_VIDEO_INDEX][ch->channel_id] = 1; if ((VPIF_CHANNEL0_VIDEO == ch->channel_id)) { channel0_intr_assert(); channel0_intr_enable(1);
- The fist part contains the email header and the first part of the email body, with the patch description, followed by Signed-off/Reviewed-by/Acked-By tags.
Note: For patches submitted by one person, but authored by another one, the first line of the email body should contain a From: tag with the name and email of the original author. For example, if the above e-mail was sent by Patch Reviewer, it would contain, instead:
From: Patch Reviewer Date: Thu, 27 Sep 2012 05:32:52 -0300 Subject: [PATCH] davinci: vpif: capture/display: fix race condition From: Patch Developer channel_first_int[][] variable is used as a flag for the ISR, This flag was being set after enabling the interrupts, There where situations when the isr occurred even before the flag was set due to which it was causing the application hang. This patch sets channel_first_int[][] flag just before enabling the interrupt. Reported-by: Some User Signed-off-by: Patch Developer Signed-off-by: Patch Reviewer
- The second part is optional, separated by "---", contains the patch diffstat. It can also contain any notice for the patch reviewers to read. The second part won't be merged at the patch commit.
- The third part is the diff, using unified diff, -p1 format. It is typically generated by a "git diff" command.
Note: on the above, Some User, Patch Developer and Patch Reviewer should be the full email of real persons, in the form: some name <some email>.
Firmware submission
Some devices may require firmware(s) in order for the device drivers to work properly. In such cases, if the firmware is not already available, some sort of procedure is needed so that end users may make use of the driver.
In the spirit of Open Source Software (OSS) development and distribution, it is most preferable if the firmware can be provided by submitting it as open source code, licenced under the GPL. Making the firmware open source facilitates tremendous advantages when it comes to debugging problems (i.e. the concept of many eyes to spot the trouble spots). Unfortunately, very few firmwares are submitted with sources, and, to be sure, this is certainly not a mandatory requirement. Indeed, as it currently stands, most firmware are instead provided in a binary format. However, there is an associated problem with distributing firmware as a closed source "binary blob" -- namely, for it to be made available within Linux distributions, it is required that each firmware should be provided with the distribution and redistribution rights, given specifically by the device or chipset manufacturer.
Therefore, if you are a device vendor or original chipset manufacturer and wish to submit the requisite firmware for inclusion within Linux distributions, in order to do so, please email the Linux Media Mailing List (LMML), and/or to Mauro (the V4L/DVB Maintainer), sending copies of the firmware files and the appropriate license terms.
If the licensing terms are deemed acceptable for legally permitting wide redistribution of the firmware software, then the firmware files will be added at the V4L/DVB Linux-firmware git tree and submitted to the upstream Linux-firmware tree.
Note that there is no unique model for firmware licensing, but there are some examples provided within the WHENCE file and within the several LICENCE files avaiable at the tree. The most common models for non GPL'd firmwares are:
There are also some existing examples of firmwares released as Open Source Software:
After you've Submitted: What happens Next?
In conjunction with the move to the new Linux-media mailing list, V4L-DVB is now using the Patchwork tool for aggregating patches sent into the list (you can read more about it here and here). In the past, it was, unfortunately, not uncommon for patches to be overlooked and become lost on the V4L-DVB mailing lists; thus making the adoption of the patchwork tool a very welcome addition. So, provided you have correctly submitted your patch (as discussed above), the steps towards having your code adopted will have automatically been put into motion. You can, of course, check to see the status of your submission from the Patchwork webpage. If patchwork has not picked up your patch (after a reasonable period of time has elapsed), it is quite probable that your submission was incorrect for some reason; please review the information contained in the above section and try again.
After being picked up by patchwork, the first thing you should expect next is that a code review will be performed, and often this is by various parties. This may lead to a series of requests for you to fix any observed problems and require you to then resubmit your work, by repeating the same steps outlined above, until everyone is happy with the submission. In other words: wash, rinse, repeat ;)
Then, when your patch is accepted, it will initially be applied to the main linux-media git tree. Once tested and integrated, such patches are later merged into a git tree by the V4L-DVB maintainer and, upon request, periodically pulled by Linus into one of his own git trees in an intermediate step towards final inclusion into the Linux kernel.