Re: [PATCH 2/5] ath6kl: Add debugfs file for target roam table

[Kalle Valo <kvalo@qca.qualcomm.com>]

On 10/10/2011 01:43 PM, Jouni Malinen wrote:
> The new roam_table debugfs file can be used to display the current
> roam table from the target.

One comment:

> +static int ath6kl_wmi_roam_tbl_event_rx(struct wmi *wmi, u8 *datap, int len)
> +{
> +#ifdef CONFIG_ATH6KL_DEBUG
> +	struct ath6kl *ar = wmi->parent_dev;
> +	struct wmi_target_roam_tbl *tbl;
> +	u16 num_entries;
> +
> +	if (len < sizeof(*tbl))
> +		return -EINVAL;
> +
> +	tbl = (struct wmi_target_roam_tbl *) datap;
> +	num_entries = le16_to_cpu(tbl->num_entries);
> +	if (sizeof(*tbl) + num_entries * sizeof(struct wmi_bss_roam_info) > len)
> +		return -EINVAL;
> +
> +	if (ar->debug.roam_tbl == NULL ||
> +	    ar->debug.roam_tbl_len < (unsigned int) len) {
> +		kfree(ar->debug.roam_tbl);
> +		ar->debug.roam_tbl = kmalloc(len, GFP_ATOMIC);
> +		if (ar->debug.roam_tbl == NULL)
> +			return -ENOMEM;
> +	}
> +
> +	memcpy(ar->debug.roam_tbl, datap, len);
> +	ar->debug.roam_tbl_len = len;
> +
> +	if (test_bit(ROAM_TBL_PEND, &ar->flag)) {
> +		clear_bit(ROAM_TBL_PEND, &ar->flag);
> +		wake_up(&ar->event_wq);
> +	}
> +#endif /* CONFIG_ATH6KL_DEBUG */
> +
> +	return 0;
> +}

I would prefer to have the part inside ifdef in debug.c, for example
like ath6kl_debug_fwlog_event() is implemented. That way we can get rid
of the ifdef inside code and related functinality would be in the same file.

Kalle