Return-path: Received: from devils.ext.ti.com ([198.47.26.153]:52904 "EHLO devils.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755151Ab0IPPym convert rfc822-to-8bit (ORCPT ); Thu, 16 Sep 2010 11:54:42 -0400 From: "Levi, Shahar" To: Julian Calaby , Luciano Coelho CC: "linux-wireless@vger.kernel.org" Date: Thu, 16 Sep 2010 17:54:35 +0200 Subject: RE: [PATCH 04/04] wl1271: 11n Support, 11n Kconfig Configurable Message-ID: References: <1284575472-19888-1-git-send-email-shahar_levi@ti.com> <1284575472-19888-5-git-send-email-shahar_levi@ti.com> <1284578468.1569.29.camel@powerslave> In-Reply-To: Content-Type: text/plain; charset="windows-1255" MIME-Version: 1.0 Sender: linux-wireless-owner@vger.kernel.org List-ID: > -----Original Message----- > From: Julian Calaby [mailto:julian.calaby@gmail.com] > Sent: Thursday, September 16, 2010 2:39 AM > To: Luciano Coelho > Cc: Levi, Shahar; linux-wireless@vger.kernel.org > Subject: Re: [PATCH 04/04] wl1271: 11n Support, 11n Kconfig Configurable > > On Thu, Sep 16, 2010 at 05:21, Luciano Coelho > wrote: > > On Wed, 2010-09-15 at 20:31 +0200, ext Shahar Levi wrote: > >> diff --git a/drivers/net/wireless/wl12xx/wl1271_main.c > b/drivers/net/wireless/wl12xx/wl1271_main.c > >> index e89e574..8f2cea9 100644 > >> --- a/drivers/net/wireless/wl12xx/wl1271_main.c > >> +++ b/drivers/net/wireless/wl12xx/wl1271_main.c > >> @@ -2087,7 +2087,9 @@ static struct ieee80211_supported_band > wl1271_band_2ghz = { > >> ? ? ? .n_channels = ARRAY_SIZE(wl1271_channels), > >> ? ? ? .bitrates = wl1271_rates, > >> ? ? ? .n_bitrates = ARRAY_SIZE(wl1271_rates), > >> +#ifdef CONFIG_WL1271_HT > >> ? ? ? .ht_cap = WL12xx_HT_CAP, > >> +#endif > > > > Here you can use #ifdef CONFIG_WL1271_80211_HT directly, no need to > > duplicate it into a new flag. > > Another thing you might want to do could be to adjust the order of the > patches so you introduce this config option *before* you introduce the > actual code that is protected by it. At the moment, this functionality > is enabled unconditionally if this patch isn't applied, which could > potentially occur during bisection, given that John doesn't squash the > patches into one when he applies them. > > Thanks, > > -- > > Julian Calaby > > Email: julian.calaby@gmail.com > Profile: http://www.google.com/profiles/julian.calaby/ > .Plan: http://sites.google.com/site/juliancalaby/ Hi Julian, It is a good point. ".ht_cap = WL12xx_HT_CAP" in wl1271_main.c adds only on patch 03/04. I believe that adding "#ifdef CONFIG_WL1271_HT" in wl1271_main.c with early patch on code that not exist is less clear. Squashing the patches 03 and 04 to one in v2 could prevent that but it will not emphasize the configuration option. Is there are cases that the some patches from series not applies? Thanks for your review, Shahar