On Tue, 2013-10-08 at 21:25 +0100, Eugene Krasnikov wrote:
> This is a mac80211 driver for Qualcomm WCN3660/WCN3680 devices. So
> far WCN3660/WCN3680 is available only on MSM platform.
some trivia:
> diff --git a/drivers/net/wireless/ath/wcn36xx/debug.c b/drivers/net/wireless/ath/wcn36xx/debug.c
[]
> +static ssize_t read_file_bool_bmps(struct file *file, char __user *user_buf,
> + size_t count, loff_t *ppos)
> +{
> + struct wcn36xx *wcn = file->private_data;
> + struct wcn36xx_vif *vif_priv = NULL;
> + struct ieee80211_vif *vif = NULL;
> + char buf[3];
> +
> + list_for_each_entry(vif_priv, &wcn->vif_list, list) {
> + vif = container_of((void *)vif_priv,
> + struct ieee80211_vif,
> + drv_priv);
> + if (NL80211_IFTYPE_STATION == vif->type) {
> + if (vif_priv->pw_state == WCN36XX_BMPS)
> + buf[0] = '1';
> + else
> + buf[0] = '0';
> + break;
> + }
please indent this properly.
list_for_each...(...) {
statement...;
statement...;
}
[]
> +static ssize_t write_file_bool_bmps(struct file *file,
> + const char __user *user_buf,
> + size_t count, loff_t *ppos)
> +{
> + struct wcn36xx *wcn = file->private_data;
> + struct wcn36xx_vif *vif_priv = NULL;
> + struct ieee80211_vif *vif = NULL;
> +
> + char buf[32];
> + int buf_size;
> +
> + buf_size = min(count, (sizeof(buf)-1));
> + if (copy_from_user(buf, user_buf, buf_size))
> + return -EFAULT;
> +
> + switch (buf[0]) {
> + case 'y':
> + case 'Y':
> + case '1':
> + list_for_each_entry(vif_priv, &wcn->vif_list, list) {
> + vif = container_of((void *)vif_priv,
> + struct ieee80211_vif,
> + drv_priv);
> + if (NL80211_IFTYPE_STATION == vif->type) {
trivia:
Please use
if (test == CONSTANT)
Yes, I know the compiler complains if you
mistakenly leave out one of the = this way.
> +#define ADD_FILE(name, mode, fop, priv_data) \
> + do { \
> + struct dentry *d; \
> + d = debugfs_create_file(__stringify(name), \
> + mode, dfs->rootdir, \
> + priv_data, fop); \
> + dfs->file_##name.dentry = d; \
> + if (IS_ERR(d)) { \
> + wcn36xx_warn("Create the debugfs entry failed");\
missing "\n" newline
> + dfs->file_##name.dentry = NULL; \
[]
> diff --git a/drivers/net/wireless/ath/wcn36xx/dxe.c b/drivers/net/wireless/ath/wcn36xx/dxe.c
[]
> +static int wcn36xx_dxe_init_descs(struct wcn36xx_dxe_ch *wcn_ch)
> +{
> + struct wcn36xx_dxe_desc *cur_dxe = NULL;
> + struct wcn36xx_dxe_desc *prev_dxe = NULL;
> + struct wcn36xx_dxe_ctl *cur_ctl = NULL;
> + size_t size;
> + int i;
> +
> + size = wcn_ch->desc_num * sizeof(struct wcn36xx_dxe_desc);
> + wcn_ch->cpu_addr = dma_alloc_coherent(NULL, size, &wcn_ch->dma_addr,
> + GFP_KERNEL);
dma_zalloc_coherent
[]
> + memset(wcn_ch->cpu_addr, 0, size);
No longer required.
> + if (0 == i) {
too many styles mixing i == 0 and 0 == i
[]
> +int wcn36xx_dxe_allocate_mem_pools(struct wcn36xx *wcn)
> +{
> + size_t s;
mixing s and size above, please use one style/name.
> + void *cpu_addr;
> +
> + /* Allocate BD headers for MGMT frames */
> +
> + /* Where this come from ask QC */
> + wcn->mgmt_mem_pool.chunk_size = WCN36XX_BD_CHUNK_SIZE +
> + 16 - (WCN36XX_BD_CHUNK_SIZE % 8);
> +
> + s = wcn->mgmt_mem_pool.chunk_size * WCN36XX_DXE_CH_DESC_NUMB_TX_H;
> + cpu_addr = dma_alloc_coherent(NULL, s, &wcn->mgmt_mem_pool.phy_addr,
> + GFP_KERNEL);
same use of dma_zalloc_coherent(...)
> + if (!cpu_addr)
> + goto out_err;
> +
> + wcn->mgmt_mem_pool.virt_addr = cpu_addr;
> + memset(cpu_addr, 0, s);
Now unnecessary memset.
too long, stopped reading.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
