Linux Wireless

ath10k mostly follows Linux Coding Style, so read that first.

For checking the code we have a dedicated script 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$

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_<hifname>”. Use ath10k_<hifname>_priv() to get access to hif specific context.

Examples:

struct ath10k *ar = ptr;
struct ath10k_pci *ar_sdio = ath10k_pci_priv(ar);

For consistency always use the main context (struct ath10k *ar) as function parameter, don't use hif specific context.

Use goto labels err_<action> for handing error path, with <action> 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\n: %d",
            sta->addr, ret);

Try to start the warning messages with the 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 Carpenters g+ post about error paths: https://plus.google.com/u/0/106378716002406849458/posts/dnanfhQ4mHQ

Always document what spinlock/mutex/rcu actually protects. Locks should always protect data, not code flow.

Name of symbols and functions follow style <drivername>_<filename>_<symbolname>.

Example:

int ath10k_mac_start(struct ath10k *ar)

For each component use function names create/destroy for allocating and freeing something, register/unregister for initialising 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)

Multiline comment style is:

/* Foo
 * Bar
 */

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 initialisation 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);

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.

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);

Don't use void pointers.

Don't use typedef.