From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755911Ab0IQSBP (ORCPT ); Fri, 17 Sep 2010 14:01:15 -0400 Received: from cantor2.suse.de ([195.135.220.15]:39322 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755884Ab0IQSBO (ORCPT ); Fri, 17 Sep 2010 14:01:14 -0400 Date: Fri, 17 Sep 2010 10:58:38 -0700 From: Greg KH To: Tim Gardner Cc: linux-kernel@vger.kernel.org, Brett Rudley , Henry Ptasinski , Nohee Ko Subject: Re: [PATCH] staging: brcm80211: Make CFLAGS explicit Message-ID: <20100917175838.GA20101@suse.de> References: <1284736810-2940-1-git-send-email-tim.gardner@canonical.com> <20100917162142.GA30385@suse.de> <4C93A8F9.9090108@canonical.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4C93A8F9.9090108@canonical.com> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Sep 17, 2010 at 11:44:25AM -0600, Tim Gardner wrote: > On 09/17/2010 10:21 AM, Greg KH wrote: > >On Fri, Sep 17, 2010 at 09:20:10AM -0600, Tim Gardner wrote: > >>Its too easy to confuse the obscurely named WLC_LOW/WLC_HIGH CFLAG > >>macros. > > > >True, but doesn't this change how the code is being built? Why make > >this change? > > > >> > >>Signed-off-by: Tim Gardner > >>Cc: Brett Rudley > >>Cc: Henry Ptasinski > >>Cc: Nohee Ko > >>--- > >> drivers/staging/brcm80211/Makefile | 3 ++- > >> 1 files changed, 2 insertions(+), 1 deletions(-) > >> > >>diff --git a/drivers/staging/brcm80211/Makefile b/drivers/staging/brcm80211/Makefile > >>index 05a4103..7a77e7f 100644 > >>--- a/drivers/staging/brcm80211/Makefile > >>+++ b/drivers/staging/brcm80211/Makefile > >>@@ -15,8 +15,9 @@ > >> # OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN > >> # CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. > >> > >>-EXTRA_CFLAGS := -DBCMDBG -DWLC_HIGH -DSTA -DWME -DWL11N -DDBAND -DBCMDMA32 -DBCMNVRAMR -Idrivers/staging/brcm80211/sys -Idrivers/staging/brcm80211/phy -Idrivers/staging/brcm80211/util -Idrivers/staging/brcm80211/include -Wall -Wstrict-prototypes -Werror -Wmissing-prototypes > >>+EXTRA_CFLAGS := -DBCMDBG -DWME -DWL11N -DDBAND -DBCMDMA32 -DBCMNVRAMR -Idrivers/staging/brcm80211/sys -Idrivers/staging/brcm80211/phy -Idrivers/staging/brcm80211/util -Idrivers/staging/brcm80211/include -Wall -Wstrict-prototypes -Werror -Wmissing-prototypes > > > >how about 2 patches here, one to break it out into one flag per line so > >we can see what is really happening here, and then one patch to make the > >change you are asking for. > > > >> > >>+USB_CFLAGS := -DWLC_HIGH > > > >Are you sure about this? Why set this when I don't see anything ever > >using it? > > > >What problem are you trying to fix here. > > > >totally confused, > > > >greg k-h > > > > Well, when groveling through the code you can see a bunch of '#ifdef > WLC_HIGH_ONLY' which I initially assumed was defined because of > '-DWLC_HIGH' in EXTRA_CFLAGS. This is quite misleading if you don't > notice 'PCI_CFLAGS := -DWLC_LOW' a bit later in the Makefile (which > I didn't for quite awhile). Specifying both macros in EXTRA_CFLAGS > seems unnecessary. > > The macro magic happens in brcm80211/sys/wlc_cfg.h: > > /* Keep WLC_HIGH_ONLY, WLC_SPLIT for USB extension later on */ > #if !defined(WLC_LOW) > #define WLC_HIGH_ONLY > #endif > #if !defined(WLC_LOW) > #define WLC_SPLIT > #endif > > What does WLC_LOW and WLC_HIGH mean anyway? Who knows, but why create USB_CFLAGS, as that's not used by anything. Henry, Brett, Nohee?