2013-07-07 02:47:40

by Larry Finger

[permalink] [raw]
Subject: [PATCH] rtlwifi: Fix Kconfig for some corner cases

Fengguang Wu reported the following errors in the build of rtlwifi with
a random configuration:

tree: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux master
head: bcd7351e83728859833e3c5b8aae9a2816914e4b
commit: 06e7cda3ce30dadc3454539127b4cfc3888c349e rtlwifi: Rework Kconfig
date: 5 months ago
config: i386-randconfig-s05-0703

drivers/built-in.o: In function `rtl_usb_cleanup':
usb.c:(.text+0x114895): undefined reference to `usb_kill_anchored_urbs'
usb.c:(.text+0x114964): undefined reference to `usb_kill_anchored_urbs'
usb.c:(.text+0x11497c): undefined reference to `usb_kill_anchored_urbs'
drivers/built-in.o: In function `_usb_writeN_sync':
usb.c:(.text+0x114c9e): undefined reference to `usb_control_msg'
drivers/built-in.o: In function `rtl_usb_disconnect':
(.text+0x114d9c): undefined reference to `usb_put_dev'
drivers/built-in.o: In function `rtl_usb_start':
usb.c:(.text+0x114fb0): undefined reference to `usb_alloc_urb'
usb.c:(.text+0x115049): undefined reference to `usb_free_urb'
--snip--

Although I could not duplicate this result on my machine, it seemed likely
that it was due to the unusual configuration where rtlwifi was being built,
but none of its dependent drivers were selected.

The fix is to select rtlwifi only when it is needed. While making this
change, I simplified the configuration so that variable RTLWIFI is never seen
by the user. In addition, now that the rtlwifi family has six members, I
added a menu so that these drivers no longer appear in the long list of
wireless drivers, but in a separate panel.

Cc: Fengguang Wu <[email protected]>
Reported-by: Fengguang Wu <[email protected]>
Tested-by: Fengguang Wu <[email protected]>
Signed-off-by: Larry Finger <[email protected]>
---
drivers/net/wireless/rtlwifi/Kconfig | 57 +++++++++++++++++++++---------------
1 file changed, 34 insertions(+), 23 deletions(-)

diff --git a/drivers/net/wireless/rtlwifi/Kconfig b/drivers/net/wireless/rtlwifi/Kconfig
index 7253de3..7552841 100644
--- a/drivers/net/wireless/rtlwifi/Kconfig
+++ b/drivers/net/wireless/rtlwifi/Kconfig
@@ -1,27 +1,18 @@
-config RTLWIFI
- tristate "Realtek wireless card support"
- depends on MAC80211
- select FW_LOADER
+menuconfig RTL_CARDS
+ tristate "Realtek rtlwifi family of devices"
+ depends on MAC80211 && (PCI || USB)
---help---
- This is common code for RTL8192CE/RTL8192CU/RTL8192SE/RTL8723AE
- drivers. This module does nothing by itself - the various front-end
- drivers need to be enabled to support any desired devices.
+ This option will enable support for the Realtek mac80211-based
+ wireless drivers. Drivers rtl8192ce, rtl8192cu, rtl8192se, rtl8192de,
+ rtl8723eu, and rtl8188eu share some common code.

- If you choose to build as a module, it'll be called rtlwifi.
-
-config RTLWIFI_DEBUG
- bool "Debugging output for rtlwifi driver family"
- depends on RTLWIFI
- default y
- ---help---
- To use the module option that sets the dynamic-debugging level for,
- the front-end driver, this parameter must be "Y". For memory-limited
- systems, choose "N". If in doubt, choose "Y".
+if RTL_CARDS

config RTL8192CE
tristate "Realtek RTL8192CE/RTL8188CE Wireless Network Adapter"
- depends on RTLWIFI && PCI
+ depends on PCI
select RTL8192C_COMMON
+ select RTLWIFI
---help---
This is the driver for Realtek RTL8192CE/RTL8188CE 802.11n PCIe
wireless network adapters.
@@ -30,7 +21,8 @@ config RTL8192CE

config RTL8192SE
tristate "Realtek RTL8192SE/RTL8191SE PCIe Wireless Network Adapter"
- depends on RTLWIFI && PCI
+ depends on PCI
+ select RTLWIFI
---help---
This is the driver for Realtek RTL8192SE/RTL8191SE 802.11n PCIe
wireless network adapters.
@@ -39,7 +31,8 @@ config RTL8192SE

config RTL8192DE
tristate "Realtek RTL8192DE/RTL8188DE PCIe Wireless Network Adapter"
- depends on RTLWIFI && PCI
+ depends on PCI
+ select RTLWIFI
---help---
This is the driver for Realtek RTL8192DE/RTL8188DE 802.11n PCIe
wireless network adapters.
@@ -48,7 +40,8 @@ config RTL8192DE

config RTL8723AE
tristate "Realtek RTL8723AE PCIe Wireless Network Adapter"
- depends on RTLWIFI && PCI
+ depends on PCI
+ select RTLWIFI
---help---
This is the driver for Realtek RTL8723AE 802.11n PCIe
wireless network adapters.
@@ -57,7 +50,8 @@ config RTL8723AE

config RTL8188EE
tristate "Realtek RTL8188EE Wireless Network Adapter"
- depends on RTLWIFI && PCI
+ depends on PCI
+ select RTLWIFI
---help---
This is the driver for Realtek RTL8188EE 802.11n PCIe
wireless network adapters.
@@ -66,7 +60,8 @@ config RTL8188EE

config RTL8192CU
tristate "Realtek RTL8192CU/RTL8188CU USB Wireless Network Adapter"
- depends on RTLWIFI && USB
+ depends on USB
+ select RTLWIFI
select RTL8192C_COMMON
---help---
This is the driver for Realtek RTL8192CU/RTL8188CU 802.11n USB
@@ -74,7 +69,22 @@ config RTL8192CU

If you choose to build it as a module, it will be called rtl8192cu

+config RTLWIFI
+ tristate
+ select FW_LOADER
+
+config RTLWIFI_DEBUG
+ bool "Debugging output for rtlwifi driver family"
+ depends on RTLWIFI
+ default y
+ ---help---
+ To use the module option that sets the dynamic-debugging level for,
+ the front-end driver, this parameter must be "Y". For memory-limited
+ systems, choose "N". If in doubt, choose "Y".
+
config RTL8192C_COMMON
tristate
depends on RTL8192CE || RTL8192CU
default m
+
+endif
--
1.8.1.4



2013-07-07 09:42:39

by Ben Hutchings

[permalink] [raw]
Subject: Re: [PATCH] rtlwifi: Fix Kconfig for some corner cases

On Sat, 2013-07-06 at 21:47 -0500, Larry Finger wrote:
> Fengguang Wu reported the following errors in the build of rtlwifi with
> a random configuration:
>
> tree: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux master
> head: bcd7351e83728859833e3c5b8aae9a2816914e4b
> commit: 06e7cda3ce30dadc3454539127b4cfc3888c349e rtlwifi: Rework Kconfig
> date: 5 months ago
> config: i386-randconfig-s05-0703
>
> drivers/built-in.o: In function `rtl_usb_cleanup':
> usb.c:(.text+0x114895): undefined reference to `usb_kill_anchored_urbs'
> usb.c:(.text+0x114964): undefined reference to `usb_kill_anchored_urbs'
> usb.c:(.text+0x11497c): undefined reference to `usb_kill_anchored_urbs'
> drivers/built-in.o: In function `_usb_writeN_sync':
> usb.c:(.text+0x114c9e): undefined reference to `usb_control_msg'
> drivers/built-in.o: In function `rtl_usb_disconnect':
> (.text+0x114d9c): undefined reference to `usb_put_dev'
> drivers/built-in.o: In function `rtl_usb_start':
> usb.c:(.text+0x114fb0): undefined reference to `usb_alloc_urb'
> usb.c:(.text+0x115049): undefined reference to `usb_free_urb'
> --snip--
>
> Although I could not duplicate this result on my machine, it seemed likely
> that it was due to the unusual configuration where rtlwifi was being built,
> but none of its dependent drivers were selected.

I think it just requires:

CONFIG_USB=m
CONFIG_RTLWIFI=y

> The fix is to select rtlwifi only when it is needed.

I don't think so. Consider this configuration:

CONFIG_PCI=y
CONFIG_USB=m
CONFIG_RTL_CARDS=y
CONFIG_RTL8192CE=y
CONFIG_RTL8192CU=m
CONFIG_RTLWIFI=y # maximum of RTL8192CE=y and RTL8192CU=m

The same breakage occurs.

> While making this
> change, I simplified the configuration so that variable RTLWIFI is never seen
> by the user.
[...]

This will break existing configurations as RTL_CARDS defaults to n.

Perhaps you should introduce separate modules for the common PCI and USB
support code, similarly to how the rt2x00 modules are organised.

Ben.

--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


2013-07-09 02:33:18

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] rtlwifi: Fix Kconfig for some corner cases

On 07/08/2013 07:40 PM, Ben Hutchings wrote:
> On Sun, 2013-07-07 at 13:13 -0500, Larry Finger wrote:
>> On 07/07/2013 04:42 AM, Ben Hutchings wrote:
>>> On Sat, 2013-07-06 at 21:47 -0500, Larry Finger wrote:
>>>>
>>>> Although I could not duplicate this result on my machine, it seemed likely
>>>> that it was due to the unusual configuration where rtlwifi was being built,
>>>> but none of its dependent drivers were selected.
>>>
>>> I think it just requires:
>>>
>>> CONFIG_USB=m
>>> CONFIG_RTLWIFI=y
>>>
>>>> The fix is to select rtlwifi only when it is needed.
>>>
>>> I don't think so. Consider this configuration:
>>>
>>> CONFIG_PCI=y
>>> CONFIG_USB=m
>>> CONFIG_RTL_CARDS=y
>>> CONFIG_RTL8192CE=y
>>> CONFIG_RTL8192CU=m
>>> CONFIG_RTLWIFI=y # maximum of RTL8192CE=y and RTL8192CU=m
>>>
>>> The same breakage occurs.
>>
>> Ben,
>>
>> Thanks for your comments. I certainly do not claim to understand very much about
>> the whole Kbuild process.
>>
>> After setting up the above configuration manually, I ran 'make oldconfig' and
>> got back
>>
>> CONFIG_RTL_CARDS=m
>> CONFIG_RTL8192CE=m
>> CONFIG_RTL8192SE=m
>> CONFIG_RTL8192DE=m
>> CONFIG_RTL8723AE=m
>> CONFIG_RTL8188EE=m
>> CONFIG_RTL8192CU=m
>> CONFIG_RTLWIFI=m
>> CONFIG_RTLWIFI_DEBUG=y
>> CONFIG_RTL8192C_COMMON=m
>>
>> That set of configuration parameters built normally.
>
> But you can still manually set:
>
> CONFIG_PCI=y
> CONFIG_USB=m
> CONFIG_MAC80211=y # maybe you had =m here?
> CONFIG_RTL_CARDS=y
> CONFIG_RTL8192CE=y
>
> and that will select:
>
> CONFIG_RTLWIFI=y
>
>>>> While making this
>>>> change, I simplified the configuration so that variable RTLWIFI is never seen
>>>> by the user.
>>> [...]
>>>
>>> This will break existing configurations as RTL_CARDS defaults to n.
>>
>> I added a 'default y' to the RTL_CARDS stanza. I now get the following:
>>
>> finger@larrylap:~/staging> make
>> scripts/kconfig/conf --silentoldconfig Kconfig
>> *
>> * Restart config...
>> *
>> *
>> * Realtek rtlwifi family of devices
>> *
>> Realtek rtlwifi family of devices (RTL_CARDS) [M/n/?] (NEW)
>>
>> That should prevent breakage of existing configurations. It defaults to "m"
>> because mac80211 is built as a module in my configuration.
>>
>>> Perhaps you should introduce separate modules for the common PCI and USB
>>> support code, similarly to how the rt2x00 modules are organised.
>>
>> That would not be very difficult, but it would involve converting the single
>> module rtlwifi into 3 separate entities. Does that make sense to you? I'm not sure.
>
> Yes it does make sense.

I will be submitting a new patch as an RFC.

Larry



2013-07-07 18:13:10

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] rtlwifi: Fix Kconfig for some corner cases

On 07/07/2013 04:42 AM, Ben Hutchings wrote:
> On Sat, 2013-07-06 at 21:47 -0500, Larry Finger wrote:
>>
>> Although I could not duplicate this result on my machine, it seemed likely
>> that it was due to the unusual configuration where rtlwifi was being built,
>> but none of its dependent drivers were selected.
>
> I think it just requires:
>
> CONFIG_USB=m
> CONFIG_RTLWIFI=y
>
>> The fix is to select rtlwifi only when it is needed.
>
> I don't think so. Consider this configuration:
>
> CONFIG_PCI=y
> CONFIG_USB=m
> CONFIG_RTL_CARDS=y
> CONFIG_RTL8192CE=y
> CONFIG_RTL8192CU=m
> CONFIG_RTLWIFI=y # maximum of RTL8192CE=y and RTL8192CU=m
>
> The same breakage occurs.

Ben,

Thanks for your comments. I certainly do not claim to understand very much about
the whole Kbuild process.

After setting up the above configuration manually, I ran 'make oldconfig' and
got back

CONFIG_RTL_CARDS=m
CONFIG_RTL8192CE=m
CONFIG_RTL8192SE=m
CONFIG_RTL8192DE=m
CONFIG_RTL8723AE=m
CONFIG_RTL8188EE=m
CONFIG_RTL8192CU=m
CONFIG_RTLWIFI=m
CONFIG_RTLWIFI_DEBUG=y
CONFIG_RTL8192C_COMMON=m

That set of configuration parameters built normally.

>> While making this
>> change, I simplified the configuration so that variable RTLWIFI is never seen
>> by the user.
> [...]
>
> This will break existing configurations as RTL_CARDS defaults to n.

I added a 'default y' to the RTL_CARDS stanza. I now get the following:

finger@larrylap:~/staging> make
scripts/kconfig/conf --silentoldconfig Kconfig
*
* Restart config...
*
*
* Realtek rtlwifi family of devices
*
Realtek rtlwifi family of devices (RTL_CARDS) [M/n/?] (NEW)

That should prevent breakage of existing configurations. It defaults to "m"
because mac80211 is built as a module in my configuration.

> Perhaps you should introduce separate modules for the common PCI and USB
> support code, similarly to how the rt2x00 modules are organised.

That would not be very difficult, but it would involve converting the single
module rtlwifi into 3 separate entities. Does that make sense to you? I'm not sure.

Thanks again for your comments.

Larry




2013-07-09 02:34:38

by Larry Finger

[permalink] [raw]
Subject: Re: [PATCH] rtlwifi: Fix Kconfig for some corner cases

On 07/06/2013 09:47 PM, Larry Finger wrote:
> Fengguang Wu reported the following errors in the build of rtlwifi with
> a random configuration:

John,

Please drop this patch. A better one will be submitted later.

Larry



2013-07-09 00:40:20

by Ben Hutchings

[permalink] [raw]
Subject: Re: [PATCH] rtlwifi: Fix Kconfig for some corner cases

On Sun, 2013-07-07 at 13:13 -0500, Larry Finger wrote:
> On 07/07/2013 04:42 AM, Ben Hutchings wrote:
> > On Sat, 2013-07-06 at 21:47 -0500, Larry Finger wrote:
> >>
> >> Although I could not duplicate this result on my machine, it seemed likely
> >> that it was due to the unusual configuration where rtlwifi was being built,
> >> but none of its dependent drivers were selected.
> >
> > I think it just requires:
> >
> > CONFIG_USB=m
> > CONFIG_RTLWIFI=y
> >
> >> The fix is to select rtlwifi only when it is needed.
> >
> > I don't think so. Consider this configuration:
> >
> > CONFIG_PCI=y
> > CONFIG_USB=m
> > CONFIG_RTL_CARDS=y
> > CONFIG_RTL8192CE=y
> > CONFIG_RTL8192CU=m
> > CONFIG_RTLWIFI=y # maximum of RTL8192CE=y and RTL8192CU=m
> >
> > The same breakage occurs.
>
> Ben,
>
> Thanks for your comments. I certainly do not claim to understand very much about
> the whole Kbuild process.
>
> After setting up the above configuration manually, I ran 'make oldconfig' and
> got back
>
> CONFIG_RTL_CARDS=m
> CONFIG_RTL8192CE=m
> CONFIG_RTL8192SE=m
> CONFIG_RTL8192DE=m
> CONFIG_RTL8723AE=m
> CONFIG_RTL8188EE=m
> CONFIG_RTL8192CU=m
> CONFIG_RTLWIFI=m
> CONFIG_RTLWIFI_DEBUG=y
> CONFIG_RTL8192C_COMMON=m
>
> That set of configuration parameters built normally.

But you can still manually set:

CONFIG_PCI=y
CONFIG_USB=m
CONFIG_MAC80211=y # maybe you had =m here?
CONFIG_RTL_CARDS=y
CONFIG_RTL8192CE=y

and that will select:

CONFIG_RTLWIFI=y

> >> While making this
> >> change, I simplified the configuration so that variable RTLWIFI is never seen
> >> by the user.
> > [...]
> >
> > This will break existing configurations as RTL_CARDS defaults to n.
>
> I added a 'default y' to the RTL_CARDS stanza. I now get the following:
>
> finger@larrylap:~/staging> make
> scripts/kconfig/conf --silentoldconfig Kconfig
> *
> * Restart config...
> *
> *
> * Realtek rtlwifi family of devices
> *
> Realtek rtlwifi family of devices (RTL_CARDS) [M/n/?] (NEW)
>
> That should prevent breakage of existing configurations. It defaults to "m"
> because mac80211 is built as a module in my configuration.
>
> > Perhaps you should introduce separate modules for the common PCI and USB
> > support code, similarly to how the rt2x00 modules are organised.
>
> That would not be very difficult, but it would involve converting the single
> module rtlwifi into 3 separate entities. Does that make sense to you? I'm not sure.

Yes it does make sense.

Ben.

--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.