User Tools

Site Tools


en:developers:documentation:submittingpatches

Differences

This shows you the differences between two versions of the page.

Link to this comparison view

Both sides previous revision Previous revision
Next revision
Previous revision
en:developers:documentation:submittingpatches [2018/01/23 08:19]
Kalle Valo [Changelog missing] add a missing verb
en:developers:documentation:submittingpatches [2024/02/01 20:12] (current)
Jeff Johnson [Who to address] add ath12k
Line 13: Line 13:
 ===== Prior to sending patches ===== ===== Prior to sending patches =====
  
-Please **DO NOT** PGP sign patches sent to //​linux-wireless//​. The reason is that signing patches will encapsulate them into MIME and thereby mangle the patch. Also, please note that we prefer patches inline rather than attachments. ​+Please **DO NOT** PGP sign patches sent to public lists. The reason is that signing patches will encapsulate them into MIME and thereby mangle the patch. Also, please note that we prefer patches inline rather than attachments. And no HTML mail, our lists reject those automatically. 
 + 
 +And carefully read [[http://​www.infradead.org/​~dwmw2/​email.html|our email etiquette]],​ that saves everyone'​s time.
  
  
Line 23: Line 25:
 CC: linux-wireless@vger.kernel.org,​ Other Developers</​code>​ CC: linux-wireless@vger.kernel.org,​ Other Developers</​code>​
  
-For wireless-drivers patches (except mac80211_hwsim) send patches as:+For wireless drivers patches (except mac80211_hwsim) send patches as:
  
 <​code>​To:​ linux-wireless@vger.kernel.org <​code>​To:​ linux-wireless@vger.kernel.org
Line 30: Line 32:
 Where [[en/​developers/​maintainers|Maintainers]] contains the list of maintainers and mailing lists for the piece of code you are patching. //Other Developers//​ in this case can be a list of developers (or mailing lists) who you think may like to review this patch or who changed the code you are working on recently. ​ Where [[en/​developers/​maintainers|Maintainers]] contains the list of maintainers and mailing lists for the piece of code you are patching. //Other Developers//​ in this case can be a list of developers (or mailing lists) who you think may like to review this patch or who changed the code you are working on recently. ​
  
-Do note that drivers might have special requirements,​ the best is to check them from the corresponding wiki page. Here's a list for few of them:+Do note that drivers might have special requirements,​ the best is to check them from the corresponding wiki page. Here's a list for few of them:
  
   * [[en/​users/​drivers/​ath10k/​submittingpatches|ath10k]]   * [[en/​users/​drivers/​ath10k/​submittingpatches|ath10k]]
 +  * [[en/​users/​drivers/​ath11k/​submittingpatches|ath11k]]
 +  * [[en/​users/​drivers/​ath11k/​submittingpatches|ath12k]]
 ===== Checking state of patches from patchwork ===== ===== Checking state of patches from patchwork =====
  
-All wireless patches are tracked in [[https://​patchwork.kernel.org/​project/​linux-wireless/​list/​|linux-wireless patchwork project]] ​(only exception being ath10k which has its own [[https://​patchwork.kernel.org/​project/​ath10k/​list/?​state=*|ath10k patchwork project]]). From patchwork you can check the state of the patch and to whom it is assigned. Here's a quick link to see all the patches, no matter what's the state:+All wireless patches are tracked in [[https://​patchwork.kernel.org/​project/​linux-wireless/​list/​|linux-wireless patchwork project]]. From patchwork you can check the state of the patch and to whom it is assigned. Here's a quick link to see all the patches, no matter what's the state:
  
 [[https://​patchwork.kernel.org/​project/​linux-wireless/​list/?​state=*]] [[https://​patchwork.kernel.org/​project/​linux-wireless/​list/?​state=*]]
  
-Always avoid contacting maintainers directly, they get way too much email already. Instead use the link above to find your patch and see the status. Only in last resort contact the maintainers,​ and do that by replying to your own patch and ask for status.+Always avoid contacting maintainers directly, they get way too much email already. Instead use the link above to find your patch and see the status. Only in last resort contact the maintainers,​ and do that by replying to your own patch and ask for status. ​Do not top post!
  
 Different patchwork states and their meanings: Different patchwork states and their meanings:
Line 56: Line 60:
 ===== Subject ===== ===== Subject =====
  
-If what you are sending is a patch you can use a subject as follows: ​+If what you are sending is a patch you should ​use a subject as follows: ​
  
  
-<​code>​[PATCH] subsystem: fix foo and optimize bar</​code>​+<​code>​[PATCH] ​wifi: subsystem: fix foo and optimize bar</​code>​ 
 + 
 +Starting from 2022 we prefix all wireless patch titles with "wifi: ".
  
 In case of wireless patches the subsystem can for example be ''​mac80211'',​ ''​cfg80211''​ or the name of the driver, for example ''​ath9k''​. There'​s an easy way to check with git what prefix you should use: In case of wireless patches the subsystem can for example be ''​mac80211'',​ ''​cfg80211''​ or the name of the driver, for example ''​ath9k''​. There'​s an easy way to check with git what prefix you should use:
Line 80: Line 86:
 If your patch is just a proposal you can mark the patch as RFC in the subject: ​ If your patch is just a proposal you can mark the patch as RFC in the subject: ​
  
-<​code>​[RFC] subsystem: a new way to do foo</​code>​+<​code>​[RFC] ​wifi: subsystem: ​add a new way to do foo</​code>​
  
 If you need to make changes to the patch add a version number inside the brackets: If you need to make changes to the patch add a version number inside the brackets:
  
-<​code>​[PATCH v2] subsystem: fix foo and optimize bar +<​code>​[PATCH v2] wifi: subsystem: fix foo and optimize bar 
-[PATCH v3] subsystem: fix foo and optimize bar +[PATCH v3] wifi: subsystem: fix foo and optimize bar 
-[PATCH v4] subsystem: fix foo and optimize bar</​code>​+[PATCH v4] wifi: subsystem: fix foo and optimize bar</​code>​
  
 **Always** increase the version number, no matter how small the change is. The maintainers focus on the latest version and ignore the older versions. Make sure that the maintainers don't need to guess what version he should take, that just creates problems. **Always** increase the version number, no matter how small the change is. The maintainers focus on the latest version and ignore the older versions. Make sure that the maintainers don't need to guess what version he should take, that just creates problems.
Line 93: Line 99:
  
 If a patch in a bigger patchset changes resubmit the whole patchset, even the patches which have not changes. The maintainers look at patchsets as a complete unit, usually they do not want to take patches individually from a patchset. If a patch in a bigger patchset changes resubmit the whole patchset, even the patches which have not changes. The maintainers look at patchsets as a complete unit, usually they do not want to take patches individually from a patchset.
 +
 +Subject lines, like commit messages (see below) should be written in imperative voice ("fix foo and optimize bar"), not in any other way such as past tense ("​fixed foo and optimized bar").
 +
 +===== Commit Messages =====
 +
 +Please write commit messages, like mentioned for the subject above, in imperative voice.
 +
 +Commit messages should describe
 +  * why a change was made,
 +  * how it achieves its stated goal, and,
 +  * if applicable, other considerations such as
 +    * alternatives that were considered,
 +    * implications on other code,
 +    * possible security implications,​
 +    * etc.
 +
 +If you find yourself listing out a number of changes in the commit message as a bulleted list or similar, consider splitting up the patch into discrete changes that each do one thing. Similarly, if one of the additional considerations is refactoring,​ try to shift that into a separate patch.
 +
 +===== Tree labels =====
 +
 +Labeling patches with what tree the patch should go to helps maintainers to prioritise and sort patches and avoids unnecessary emails, which saves everyone time and speeds up patch review. Here are some tips how to label wireless patches.
 +
 +If you want to target your patch to a specific release (for example that the patch should go -rc release not -next) you can inform the maintainer by adding the release number inside the PATCH brackets:
 + 
 +<​code>​[PATCH 4.20] wifi: subsystem: fix foo</​code>​
 +
 +If you want to make it clear to the maintainer that the patch should NOT go to -rc release but to -next instead you can add "​-next"​ to PATCH brackets:
 +
 +<​code>​[PATCH -next] wifi: subsystem: fix foo</​code>​
 +
 +Alternatively you can specify the exact tree you are targetting by adding the name of the git tree inside PATCH brackets:
 +
 +<​code>​[PATCH wireless] wifi: mac80211: fix foo
 +[PATCH wireless-next] wifi: mac80211: implement very-cool-feature
 +[PATCH wireless] wifi: ath10k: fix foo
 +[PATCH wireless-next] wifi: ath10k: implement awesome-feature
 +</​code>​
 +
  
 ===== Sending large patches or multiple patches ===== ===== Sending large patches or multiple patches =====
Line 101: Line 145:
  
  
-<​code>​[PATCH 0/4] driver_name:​ introduce foo and bar +<​code>​[PATCH 0/4] wifi: driver_name:​ introduce foo and bar 
-[PATCH 1/4] driver_name:​ introduce get_foo_bars() +[PATCH 1/4] wifi: driver_name:​ introduce get_foo_bars() 
-[PATCH 2/4] driver_name:​ fix locking on bar_by_foo() +[PATCH 2/4] wifi: driver_name:​ fix locking on bar_by_foo() 
-[PATCH 3/4] driver_name:​ use foo when barring +[PATCH 3/4] wifi: driver_name:​ use foo when barring 
-[PATCH 4/4] driver_name:​ optimize bar at init time</​code>​ +[PATCH 4/4] wifi: driver_name:​ optimize bar at init time</​code>​ 
-On the e-mail with subject, "​[PATCH 0/4] driver_name:​ introduce foo and bar", you would give a brief overview of all the changes. No patch should be included in that e-mail, and as that e-mail will not end up in the change logs it should not contain anything that should be archived, only a rough overview over the purpose of the patch set, no in-depth description which should be in the changelog for each patch. ​+On the e-mail with subject, "​[PATCH 0/4] wifi: driver_name:​ introduce foo and bar", you would give a brief overview of all the changes. No patch should be included in that e-mail, and as that e-mail will not end up in the change logs it should not contain anything that should be archived, only a rough overview over the purpose of the patch set, no in-depth description which should be in the changelog for each patch. ​
  
  
 ===== Format of patches ===== ===== Format of patches =====
  
-We prefer patches to be inline-text at the end of the body of the e-mail. ​You can use git-diff or the like to generate ​the patch. Additionally note that we prefer to apply patches with -p1. A header as follows is then acceptable: ​+We prefer patches to be inline-text at the end of the body of the e-mail. ​It's strongly recommended to use git-format-patch and git-send-email tools to submit patches as they use the correct format automatically. Additionally note that we prefer to apply patches with git-am (using the -p1 diff format). A header as follows is then acceptable: ​
  
  
Line 134: Line 178:
  
  
 +===== New driver =====
 +
 +For submitting a new wireless driver the requirements are:
 +
 +  * follow [[https://​www.kernel.org/​doc/​html/​latest/​process/​coding-style.html|Linux kernel coding style]]
 +  * use [[https://​www.kernel.org/​doc/​html/​latest/​process/​license-rules.html|SPDX tags]]
 +  * use either cfg80211 or mac80211, depending on the firmware architecture (no custom 802.11 stack in the driver)
 +  * have firmware images submitted for [[https://​git.kernel.org/​pub/​scm/​linux/​kernel/​git/​firmware/​linux-firmware.git/​|linux-firmware]] with an acceptable license allowing redistribution
 +  * document Device Tree usage in [[https://​www.kernel.org/​doc/​html/​latest/​devicetree/​bindings/​submitting-patches.html|devicetree bindings]] and review them with DT maintainers
 +  * in the commit log/cover letter provide an overview of the driver
 +    * what hardware the driver supports
 +    * what features are supported (client, AP, mesh modes etc)
 +  * for review submit the driver as one file per patch, to make it easier for the reviewers
 +    * example: https://​lore.kernel.org/​linux-wireless/​20200623110000.31559-1-ajay.kathat@microchip.com/​
 +  * final commit (after the review) will be one big patch
 +    * for staging drivers the final patch will be just a small patch moving the driver, example: https://​git.kernel.org/​linus/​5625f965d764
 +
 +There'​s also a list of [[https://​git.kernel.org/​pub/​scm/​linux/​kernel/​git/​torvalds/​linux.git/​tree/​LICENSES/​preferred|preferred licenses]] available.
 +
 +Some guidelines to speed up new driver review:
 +
 +  * keep the driver small and simple, more features can be added after the driver is accepted upstream
 +  * use clean understandable code
 +  * use generic kernel frameworks instead of reinventing the wheel
 +  * use generic user space interfaces
 +    * no driver specific user interfaces or hacks
 +    * no .ini style driver configuration files
 +  * avoid using debugfs or nl80211 vendor interfaces
 ===== Examples of a patches ===== ===== Examples of a patches =====
  
Line 141: Line 213:
 To: John Linville To: John Linville
 Cc: linux-wireless,​ Bcm43xx-dev,​ Larry Finger Cc: linux-wireless,​ Bcm43xx-dev,​ Larry Finger
-Subject: [PATCH] b43: Remove the "radio hw enabled"​ message on startup.+Subject: [PATCH] ​wifi: b43: Remove the "radio hw enabled"​ message on startup.
  
 This message is useless. Only report state changes. This message is useless. Only report state changes.
Line 179: Line 251:
 To: John Linville To: John Linville
 Cc: linux-wireless,​ Michael Wu, Johannes Berg, Daniel Drake, Larry Finger Cc: linux-wireless,​ Michael Wu, Johannes Berg, Daniel Drake, Larry Finger
-Subject: [PATCH 3/5] Wireless: add IEEE-802.11 regualtory domain module+Subject: [PATCH 3/5] wifi: add IEEE-802.11 regualtory domain module
  
 This adds the regulatory domain module. It provides a way to This adds the regulatory domain module. It provides a way to
Line 220: Line 292:
 If you send a new version of the patch or patchset you should always add a version number. The first version does not need to be shown but starting from second version the version number must be available: If you send a new version of the patch or patchset you should always add a version number. The first version does not need to be shown but starting from second version the version number must be available:
  
-  [PATCH] ath10k: fix DMA allocation +  [PATCH] ​wifi: ath10k: fix DMA allocation 
-  [PATCH v2] ath10k: fix DMA allocation +  [PATCH v2] wifi: ath10k: fix DMA allocation 
-  [PATCH v3] ath10k: fix DMA allocation+  [PATCH v3] wifi: ath10k: fix DMA allocation
   ...   ...
-  [PATCH v11] ath10k: fix DMA allocation+  [PATCH v11] wifi: ath10k: fix DMA allocation
  
 You can add the version with switch ''​--subject-prefix'':​ You can add the version with switch ''​--subject-prefix'':​
Line 242: Line 314:
 ==== Signed-off-by missing ==== ==== Signed-off-by missing ====
  
-Read [[https://​www.kernel.org/​doc/​html/​latest/​process/​submitting-patches.html#​sign-your-work-the-developer-s-certificate-of-origin|Developer'​s Certificate of Origin]] and add Signed-off-by ​to the commit log.+Read [[https://​www.kernel.org/​doc/​html/​latest/​process/​submitting-patches.html#​sign-your-work-the-developer-s-certificate-of-origin|Developer'​s Certificate of Origin]]. Do not submit patches unless you have read, understood ​and agreed ​to the certificate. 
  
 ==== Format issues ==== ==== Format issues ====
Line 256: Line 329:
 commit log message header line text enclosed in parentheses and commit log message header line text enclosed in parentheses and
 double quotes with no line breaks whatsoever. double quotes with no line breaks whatsoever.
 +The fixes lines must be placed just above the signed-off-by lines.
  
 Example: Example:
Line 261: Line 335:
   Fixes: c742e623e941 ("​mwifiex:​ sdio card reset enhancement"​)   Fixes: c742e623e941 ("​mwifiex:​ sdio card reset enhancement"​)
  
 +Here's how one can configure git to provide the fixes tag in correct format:
 +
 +  $ git config --global --add alias.fixes 'show -q --format=fixes'​
 +  $ git config --global --add pretty.fixes '​Fixes:​ %h ("​%s"​)'​
 +  $ git config --global --add core.abbrev 12
 +  $ git fixes ba9177fcef21
 +  Fixes: ba9177fcef21 ("​ath11k:​ Add basic WoW functionalities"​)
 ==== Commit reference is wrong ==== ==== Commit reference is wrong ====
  
Line 287: Line 368:
 ==== Commit title is wrong ==== ==== Commit title is wrong ====
  
-The correc tformat ​for the commit title is name of driver, followed by a colon, followed by a space and then followed by the actual title. ​You can use ''​git log''​ to check older commits and see what prefix was used:+The correct format ​for the commit title is name of driver, followed by a colon, followed by a space and then followed by the actual title. ​Also the title should be informative and unique, so something like "fix a bug" is not a good title. 
 + 
 +In 2022 we started using "wifi: " in front of all wireless patches. 
 + 
 +For examples uou can use ''​git log''​ to check older commits and see what prefix was used:
  
 <​code>​ <​code>​
Line 321: Line 406:
 The commit log needs to tell why you wrote the patch. If you fixed a bug give a short summary of the bug (can be a long one as well, of course) from user's point of view, and if there'​s a publically available bug report include a link to that. If you are fixing a warning from a compiler or a static checker add the warning from tool. Or if it's just code cleanup or fixing a theoretical issue, and does not have practical user visible changes, mention that also. The commit log needs to tell why you wrote the patch. If you fixed a bug give a short summary of the bug (can be a long one as well, of course) from user's point of view, and if there'​s a publically available bug report include a link to that. If you are fixing a warning from a compiler or a static checker add the warning from tool. Or if it's just code cleanup or fixing a theoretical issue, and does not have practical user visible changes, mention that also.
  
-===== More patch work references =====+==== Do not top post and edit your quotes ​==== 
 + 
 +Top posting makes following email threads hard to follow and also it makes use of patchwork more difficult, which gets the maintainers grumpy as you are making their work more difficult. So do not top post and instead edit your quotes properly. 
 + 
 +<​code>​A:​ Because it messes up the order in which people normally read text. 
 +Q: Why is top-posting such a bad thing? 
 +A: Top-posting. 
 +Q: What is the most annoying thing in e-mail? 
 + 
 +A: No. 
 +Q: Should I include quotations after my reply? 
 +</​code>​ 
 + 
 +More info: http://​www.idallen.com/​topposting.html 
 + 
 +==== Do not send HTML mail ==== 
 + 
 +linux-wireless mailing list drops all mail using HTML, so don't use it. 
 + 
 +==== Use RFC or RFT for patches not ready ==== 
 + 
 +If the patches are not yet ready to be applied by the maintainer, mark them as RFC (Request For Comments) or RFT (Request For Test) in the subject. This way the maintainer can easily see that the patch should not be applied yet and saves maintainer'​s time. 
 + 
 +Examples: 
 + 
 +  [PATCH RFC] wifi: ath11k: enable power save mode always 
 +  [PATCH RFT] wifi: ath10k: sdio: always use DMA transfers 
 + 
 + 
 +==== Use Co-developed-by when multiple authors ​ ==== 
 + 
 +When a patch has multiple authors you should use Co-developed-by tag: 
 + 
 +https://​www.kernel.org/​doc/​html/​latest/​process/​submitting-patches.html#​when-to-use-acked-by-cc-and-co-developed-by 
 + 
 +==== Maximum of 7-12 patches per patchset ​ ==== 
 + 
 +If you want your patches reviewed smoothly submit maximum of 7-12 patches per patchset. If the patches are bigger don't send more than 7 patches. But if they smaller, or trivial patches, 12 patches is ok. But anything more than 12 patches and you will get reviewers grumpy (read: it takes longer to get your patches reviewed and applied). 
 + 
 +But you can submit multiple patchsets, just try to throttle it down to avoid bufferbloat in patchwork, for example you can send a new patchset every other day. And don't forget to document the dependencies in the cover letter ("this patchset depends on patchset B"). 
 + 
 +===== More references =====
  
 Here is a list of links to help you write better patches ​ Here is a list of links to help you write better patches ​
Line 329: Line 455:
   * [[http://​linux.yyz.us/​patch-format.html|http://​linux.yyz.us/​patch-format.html]] ​   * [[http://​linux.yyz.us/​patch-format.html|http://​linux.yyz.us/​patch-format.html]] ​
   * [[https://​www.ozlabs.org/​~akpm/​stuff/​tpp.txt|Andrew Morton'​s ''​The perfect patch''​]] ​   * [[https://​www.ozlabs.org/​~akpm/​stuff/​tpp.txt|Andrew Morton'​s ''​The perfect patch''​]] ​
 +  * [[http://​lkml.kernel.org/​r/​20171026223701.GA25649@bhelgaas-glaptop.roam.corp.google.com|Make Bjorn'​s life easier (and grease the path of your patch)]]
en/developers/documentation/submittingpatches.1516695574.txt.gz · Last modified: 2018/01/23 08:19 (external edit)