Linux wireless drivers development
 help / color / mirror / Atom feed
From: Joe Perches <joe@perches.com>
To: Kalle Valo <kvalo@qca.qualcomm.com>
Cc: linux-wireless@vger.kernel.org, devel@linuxdriverproject.org,
	gregkh@suse.de
Subject: Re: [PATCH 03/24] ath6kl: add cfg80211.c
Date: Tue, 12 Jul 2011 21:29:03 -0700	[thread overview]
Message-ID: <1310531343.1143.26.camel@Joe-Laptop> (raw)
In-Reply-To: <20110713013324.8517.86411.stgit@localhost6.localdomain6>

On Wed, 2011-07-13 at 04:33 +0300, Kalle Valo wrote: 
> Signed-off-by: Kalle Valo <kvalo@qca.qualcomm.com>
> diff --git a/drivers/net/wireless/ath/ath6kl/cfg80211.c b/drivers/net/wireless/ath/ath6kl/cfg80211.c
[] 
> +static struct ieee80211_rate ath6kl_rates[] = {

const?

> +static struct ieee80211_channel ath6kl_2ghz_channels[] = {

const?

> +static struct ieee80211_channel ath6kl_5ghz_a_channels[] = {

const?

> +static struct ieee80211_supported_band ath6kl_band_2ghz = {

const?

> +static struct ieee80211_supported_band ath6kl_band_5ghz = {

const?

> +		ath6kl_err("%s: %u not spported\n", __func__, wpa_version);
> +		ath6kl_err("%s: 0x%x not spported\n", __func__, auth_type);

supported

> +		wait_event_interruptible_timeout(ar->event_wq,
> +						 ar->
> +						 tx_pending[wmi_get_control_ep
> +							     (ar->wmi)] == 0,
> +						 WMI_TIMEOUT);

I think you can ignore 80 columns for this.

		wait_event_interruptible_timeout(ar->event_wq,
						 ar->tx_pending[wmi_get_control_ep(ar->wmi)] == 0,
						 WMI_TIMEOUT);

or maybe use different argument alignment when appropriate:

		wait_event_interruptible_timeout(ar->event_wq,
			ar->tx_pending[wmi_get_control_ep(ar->wmi)] == 0,
			WMI_TIMEOUT);

[]

> +		if (sme->key_idx < WMI_MIN_KEY_INDEX
> +		    || sme->key_idx > WMI_MAX_KEY_INDEX) {

I think it's more consistent with other files in drivers/net
to use logical continuations at EOL

if (sme->key_idx < WMI_MIN_KEY_INDEX ||
    sme->key_idx > WMI_MAX_KEY_INDEX) {

> +			ath6kl_err("%s: key index %d out of bounds\n", __func__,
> +				   sme->key_idx);

I think it's better to use format on one line,
arguments on another.

			ath6kl_err("%s: key index %d out of bounds\n",
				    __func__, sme->key_idx);


It seems almost all of your ath6kl_<level> logging
messages use __func__.  Perhaps you might as well
#define it to take __func__ instead of using it
in-place over and over.




  reply	other threads:[~2011-07-13  4:29 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-07-13  1:32 [PATCH 00/24] ath6kl cleaned up driver Kalle Valo
2011-07-13  1:33 ` [PATCH 01/24] ath6kl: add bmi.c Kalle Valo
2011-07-13  4:28   ` Joe Perches
2011-07-13  7:08     ` Kalle Valo
2011-07-13 12:19       ` Joe Perches
2011-07-14  6:46         ` Kalle Valo
2011-07-14  7:33           ` Joe Perches
2011-07-14  7:38             ` Kalle Valo
2011-07-13  1:33 ` [PATCH 02/24] ath6kl: add bmi.h Kalle Valo
2011-07-13  1:33 ` [PATCH 03/24] ath6kl: add cfg80211.c Kalle Valo
2011-07-13  4:29   ` Joe Perches [this message]
2011-07-13  7:22     ` Kalle Valo
2011-07-13  1:33 ` [PATCH 04/24] ath6kl: add cfg80211.h Kalle Valo
2011-07-13  1:33 ` [PATCH 05/24] ath6kl: add common.h Kalle Valo
2011-07-13  1:33 ` [PATCH 06/24] ath6kl: add core.h Kalle Valo
2011-07-13  4:29   ` Joe Perches
2011-07-13  7:41     ` Kalle Valo
2011-07-13  1:34 ` [PATCH 07/24] ath6kl: add debug.c Kalle Valo
2011-07-13  4:29   ` Joe Perches
2011-07-13  1:34 ` [PATCH 08/24] ath6kl: add debug.h Kalle Valo
2011-07-13  1:34 ` [PATCH 09/24] ath6kl: add hif-ops.h Kalle Valo
2011-07-13  1:34 ` [PATCH 10/24] ath6kl: add hif.h Kalle Valo
2011-07-13  1:34 ` [PATCH 11/24] ath6kl: add htc.c Kalle Valo
2011-07-13  4:30   ` Joe Perches
2011-07-13  7:46     ` Kalle Valo
2011-07-13 12:33       ` Joe Perches
2011-07-13 13:38         ` Vasanthakumar Thiagarajan
2011-07-13  1:34 ` [PATCH 12/24] ath6kl: add htc.h Kalle Valo
2011-07-13  1:35 ` [PATCH 13/24] ath6kl: add htc_hif.c Kalle Valo
2011-07-13  1:35 ` [PATCH 14/24] ath6kl: add htc_hif.h Kalle Valo
2011-07-13  1:35 ` [PATCH 15/24] ath6kl: add ieee80211.h Kalle Valo
2011-07-13  7:29   ` Johannes Berg
2011-07-13  8:25     ` Kalle Valo
2011-07-13  1:35 ` [PATCH 16/24] ath6kl: add init.c Kalle Valo
2011-07-13  1:35 ` [PATCH 17/24] ath6kl: add main.c Kalle Valo
2011-07-13  4:30   ` Joe Perches
2011-07-13  8:18     ` Kalle Valo
2011-07-13  1:35 ` [PATCH 18/24] ath6kl: add node.c Kalle Valo
2011-07-13  1:36 ` [PATCH 19/24] ath6kl: add sdio.c Kalle Valo
2011-07-13  1:36 ` [PATCH 20/24] ath6kl: add target.h Kalle Valo
2011-07-13  1:36 ` [PATCH 21/24] ath6kl: add txrx.c Kalle Valo
2011-07-13  4:31   ` Joe Perches
2011-07-13  8:21     ` Kalle Valo
2011-07-13  8:23       ` Kalle Valo
2011-07-13  1:36 ` [PATCH 22/24] ath6kl: add wmi.c Kalle Valo
2011-07-13  1:36 ` [PATCH 23/24] ath6kl: add wmi.h Kalle Valo
2011-07-13  1:37 ` [PATCH 24/24] ath6kl: add Kconfig and Makefile Kalle Valo
2011-07-13  8:34 ` [PATCH 00/24] ath6kl cleaned up driver Kalle Valo
2011-07-13  9:22 ` Dan Carpenter
2011-07-13  9:28   ` Johannes Berg
2011-07-13  9:34     ` Dan Carpenter
2011-07-13  9:41       ` Johannes Berg
2011-07-13  9:33   ` Dan Carpenter
2011-07-13 10:04   ` Vasanthakumar Thiagarajan
2011-07-14 10:06 ` Dan Carpenter
2011-07-14 10:35   ` Kalle Valo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1310531343.1143.26.camel@Joe-Laptop \
    --to=joe@perches.com \
    --cc=devel@linuxdriverproject.org \
    --cc=gregkh@suse.de \
    --cc=kvalo@qca.qualcomm.com \
    --cc=linux-wireless@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox