This shows you the differences between two versions of the page.
Next revision | Previous revision Next revision Both sides next revision | ||
en:users:drivers:ath10k:codingstyle [2015/01/26 09:49] 127.0.0.1 external edit |
en:users:drivers:ath10k:codingstyle [2018/04/13 11:16] Kalle Valo Add section for error message |
||
---|---|---|---|
Line 7: | Line 7: | ||
===== ath10k Coding Style ===== | ===== ath10k Coding Style ===== | ||
+ | |||
+ | ==== Linux coding style ==== | ||
+ | |||
+ | ath10k mostly follows [[https://www.kernel.org/doc/html/latest/process/coding-style.html|Linux Coding Style]], so read that first. | ||
+ | |||
+ | ==== Checking code ==== | ||
+ | |||
+ | For checking the code we have a dedicated script [[https://github.com/qca/qca-swiss-army-knife/blob/master/tools/scripts/ath10k/ath10k-check|ath10k-check]] which runs various tests, including sparse and checkpatch. Run the script with ''--help'' to see the installation and usage instructions. Strongly recommended to run this before submitting patches as it can catch common problems. Example: | ||
+ | |||
+ | <code> | ||
+ | ~$ cd ~/ath | ||
+ | ~/ath$ ls | ||
+ | arch/ debian/ include/ lib/ Module.symvers sound/ | ||
+ | block/ Documentation/ init/ localversion-wireless-testing net/ tools/ | ||
+ | certs/ drivers/ ipc/ localversion-wireless-testing-ath README usr/ | ||
+ | COPYING firmware/ Kbuild MAINTAINERS samples/ virt/ | ||
+ | CREDITS fs/ Kconfig Makefile scripts/ vmlinux-gdb.py@ | ||
+ | crypto/ GNUmakefile kernel/ mm/ security/ | ||
+ | ~/ath$ ath10k-check | ||
+ | drivers/net/wireless/ath/ath10k/debug.h:207: return is not a function, parentheses are not required | ||
+ | drivers/net/wireless/ath/ath10k/debug.h:209: return is not a function, parentheses are not required | ||
+ | drivers/net/wireless/ath/ath10k/debug.h:210: return is not a function, parentheses are not required | ||
+ | drivers/net/wireless/ath/ath10k/debug.h:214: Alignment should match open parenthesis | ||
+ | drivers/net/wireless/ath/ath10k/debug.h:218: Alignment should match open parenthesis | ||
+ | drivers/net/wireless/ath/ath10k/debug.c:2430: code indent should use tabs where possible | ||
+ | drivers/net/wireless/ath/ath10k/debug.c:2430: please, no spaces at the start of a line | ||
+ | drivers/net/wireless/ath/ath10k/debug.c:2431: code indent should use tabs where possible | ||
+ | drivers/net/wireless/ath/ath10k/debug.c:2431: please, no spaces at the start of a line | ||
+ | drivers/net/wireless/ath/ath10k/debug.c:2464: code indent should use tabs where possible | ||
+ | drivers/net/wireless/ath/ath10k/debug.c:2464: please, no spaces at the start of a line | ||
+ | drivers/net/wireless/ath/ath10k/debug.c:2465: code indent should use tabs where possible | ||
+ | drivers/net/wireless/ath/ath10k/debug.c:2465: please, no spaces at the start of a line | ||
+ | drivers/net/wireless/ath/ath10k/debug.c:2493: Please don't use multiple blank lines | ||
+ | drivers/net/wireless/ath/ath10k/debug.c:2525: Symbolic permissions 'S_IRUSR' are not preferred. Consider using octal permissions '0400'. | ||
+ | drivers/net/wireless/ath/ath10k/debug.c:2527: Symbolic permissions 'S_IRUSR' are not preferred. Consider using octal permissions '0400'. | ||
+ | drivers/net/wireless/ath/ath10k/debug.c:2620: Alignment should match open parenthesis | ||
+ | drivers/net/wireless/ath/ath10k/debug.c:2640: Alignment should match open parenthesis | ||
+ | ~/ath$ | ||
+ | </code> | ||
==== Status/error variables ==== | ==== Status/error variables ==== | ||
Line 87: | Line 126: | ||
- | <code>int ath10k_init_hw(struct ath6kl *ar)</code> | + | <code>int ath10k_init_hw(struct ath10k *ar)</code> |
For each component use function names create/destroy for allocating and freeing something, init/cleanup for initialising variables and cleaning up them afterwards and start/stop to temporarily pause something. | For each component use function names create/destroy for allocating and freeing something, init/cleanup for initialising variables and cleaning up them afterwards and start/stop to temporarily pause something. | ||
Line 107: | Line 146: | ||
*/</code> | */</code> | ||
- | ==== Things NOT to do ==== | + | ==== Error messages ==== |
- | Don't use void pointers. | + | For warning and error messages we have ath10k_warn() and ath10k_err(). |
- | Don't use typedef. | + | ath10k_warn() should be used when ath10k still continues to work, for example then some limit has been reached or an unknown event has been received. It's also rate limited. |
+ | ath10k_err() should be used when a fatal error has been detected and ath10k will shut itself down, for example during driver initialisation or firmware recover fails. It is NOT rate limited. | ||
- | ==== Linux style ==== | + | ==== Debug messages ==== |
- | Follow [[http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=Documentation/CodingStyle;hb=HEAD|Linux Coding Style]]. | + | Use ath10k_dbg() or ath10k_dbg_dump(). |
+ | The format string for ath10k_dbg() should start with debug level followed by name of the command or event and then parameters. All uppercase and no commas, colons or periods. | ||
- | ==== Checking code ==== | + | Examples: |
- | Run sparse: | + | <code>ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot suspend complete\n"); |
+ | ath10k_dbg(ar, ATH10K_DBG_WMI, "wmi mgmt tx skb %pK len %d ftype %02x stype %02x\n", | ||
+ | msdu, skb->len, fc & IEEE80211_FCTL_FTYPE, | ||
+ | fc & IEEE80211_FCTL_STYPE); | ||
+ | |||
+ | ath10k_dbg(ar, ATH10K_DBG_MAC, "mac update sta %pM peer bw %d\n", | ||
+ | sta->addr, bw); | ||
+ | </code> | ||
+ | ==== Things NOT to do ==== | ||
+ | |||
+ | Don't use void pointers. | ||
+ | |||
+ | Don't use typedef. | ||
- | <code>make drivers/net/wireless/ath/ath10k/ C=2 CF="-D__CHECK_ENDIAN__"</code> |