[PATCH2 4/6] staging: brcm80211: assure common sources are truly common

[Arend Van Spriel]

Hi Greg,

Thanks for clarifying which context to keep in mind when adding Kconfig options. I looked at it like
something that also developers like me could use to enable more debugging features for our driver
like "Kernel hacking" does in general.

I will rework these patches and resend them.

Gr. AvS
________________________________________
From: Greg KH [greg at kroah.com]
Sent: Friday, January 21, 2011 9:02 PM
To: Arend Van Spriel
Cc: gregkh at suse.de; devel at linuxdriverproject.org
Subject: Re: [PATCH2 4/6] staging: brcm80211: assure common sources are truly common

On Fri, Jan 21, 2011 at 10:54:51AM +0100, Arend van Spriel wrote:
> Common code for brcm80211 drivers was resulting in different compiled
> object files for the drivers due to compilation flags. This has been
> aligned so that they are resulting in same object files. Kconfig now
> allows both drivers to be build simultaneously.
>
> Reviewed-by: Brett Rudley <brudley at broadcom.com>
> Reviewed-by: Henry Ptasinski <henryp at broadcom.com>
> Reviewed-by: Roland Vossen <rvossen at broadcom.com>
> Signed-off-by: Arend van Spriel <arend at broadcom.com>
> ---
>  drivers/staging/brcm80211/Kconfig           |   21 ++++++++++++++-------
>  drivers/staging/brcm80211/Makefile          |    6 ++++--
>  drivers/staging/brcm80211/brcmfmac/Makefile |    9 ++++++---
>  drivers/staging/brcm80211/brcmsmac/Makefile |    3 +--
>  drivers/staging/brcm80211/util/aiutils.c    |    4 ----
>  drivers/staging/brcm80211/util/bcmutils.c   |    1 +
>  drivers/staging/brcm80211/util/hnddma.c     |    1 +
>  drivers/staging/brcm80211/util/hndpmu.c     |   14 ++++++++------
>  8 files changed, 35 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/staging/brcm80211/Kconfig b/drivers/staging/brcm80211/Kconfig
> index 3208352..190e7e4 100644
> --- a/drivers/staging/brcm80211/Kconfig
> +++ b/drivers/staging/brcm80211/Kconfig
> @@ -2,12 +2,6 @@ menuconfig BRCM80211
>       tristate "Broadcom IEEE802.11n WLAN drivers"
>       depends on WLAN
>
> -choice
> -     prompt "Broadcom IEEE802.11n driver style"
> -     depends on BRCM80211
> -     help
> -     Select the appropriate driver style from the list below.
> -
>  config BRCMSMAC
>       bool "Broadcom IEEE802.11n PCIe SoftMAC WLAN driver"
>       depends on PCI
> @@ -30,4 +24,17 @@ config BRCMFMAC
>         Broadcom IEEE802.11n FullMAC chipsets.  This driver uses the kernel's
>         wireless extensions subsystem.  If you choose to build a module,
>         it'll be called brcmfmac.ko.
> -endchoice
> +
> +config BRCMDBG
> +     bool "Broadcom driver debug functions"
> +     default y

No new feature should EVER default to y unless you can not boot your
machine with it.

> +     depends on BRCM80211
> +     ---help---
> +       Selecting this enables additional code for debug purposes.
> +
> +config BRCMDBG_ASSERT
> +     bool "Broadcom ASSERT debugging function"

Ick ick ick.  Is this really something that should be a config option?
Why would any user ever want to enable this option?

> +     depends on BRCMDBG
> +     ---help---
> +       Selecting this assures assertions in driver are more verbose and

You are adding trailing spaces in your patch.  Have you not run it
through checkpatch.pl?

Sorry, I can't take this one.  I took the first 3 though.  Care to
resend these next 3?

thanks,

greg k-h