Go back –> [[en/users/Drivers/ath10k|ath10k]] ===== 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: ~$ 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$ ==== Status/error variables ==== Use a variable named "ret" to store return values or status codes. Also propagate the error code to upper levels. Example: int ret; ret = request_firmware(&fw_entry, filename, ar->dev); if (ret) { ath10k_warn("Failed to request firmware '%s': %d\n", filename, ret); return ret; } return 0; Name context variables either "ar" or "ar_". Use ath10k__priv() to get access to hif specific context. Examples: struct ath10k *ar = ptr; struct ath10k_pci *ar_pci = ath10k_pci_priv(ar); For consistency always use the main context (struct ath10k *ar) as function parameter, don't use hif specific context. ==== Error path ==== Use goto labels err_ for handing error path, with giving a clear idea what the label does. Example: ret = ath10k_hif_power_on(ar); if (ret) return ret; ret = ath10k_target_start(ar); if (ret) goto err_power_off; ret = ath10k_init_upload(ar); if (ret) goto err_target_stop; return 0; err_target_stop: ath10k_target_stop(ar); err_power_off: ath10k_hif_power_off(ar); return ret; Print error codes after a colon: ath10k_warn("failed to associate peer STA %pM: %d\n", sta->addr, ret); Try to start the warning messages with the verb "failed" if possible. Warning and error messages start with lower case. ath10k_warn() is used for errors where it might be possible to recover and ath10k_err() for errors when it's not possible to recover in any way. Dan Carpenter's post about error paths: [[https://staticthinking.wordpress.com/2022/04/28/free-the-last-thing-style/|https://staticthinking.wordpress.com/2022/04/28/free-the-last-thing-style/]] ==== Locking ==== Always document what spinlock/mutex/rcu actually protects. Locks should always protect data, not code flow. ==== Naming ==== Name of symbols and functions follow style __. Example: int ath10k_mac_start(struct ath10k *ar) For each component use function names create/destroy for allocating and freeing something, register/unregister for initializing and cleaning up them afterwards and start/stop to temporarily pause something. Example: int ath10k_cfg80211_create(struct ath10k *ar) int ath10k_cfg80211_register(struct ath10k *ar) int ath10k_cfg80211_start(struct ath10k *ar) void ath10k_cfg80211_stop(struct ath10k *ar) int ath10k_cfg80211_unregister(struct ath10k *ar) void ath10k_cfg80211_destroy(struct ath10k *ar) ==== Comments ==== Multiline comment style is: /* Foo * Bar */ ==== Error messages ==== For warning and error messages we have ath10k_warn() and ath10k_err(). 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 initialization or firmware recover fails. It is NOT rate limited. Examples: ath10k_warn(ar, "failed to submit frame %d: %d\n", frame_index, ret); ath10k_err(ar, "failed to wake up the device from sleep: %d\n", ret); ==== Debug messages ==== 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 lowercase and no commas, colons or periods. Examples: 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); ==== Things NOT to do ==== Don't use void pointers. Don't use typedef.