diff mbox

Revert "ipw2200: select CFG80211_WEXT"

Message ID 1420297188.2397.3.camel@tiscali.nl (mailing list archive)
State Rejected
Delegated to: Kalle Valo
Headers show

Commit Message

Paul Bolle Jan. 3, 2015, 2:59 p.m. UTC
This reverts commit dddd60220f41775e634258efd1b54c6fa81ce706.

The raison d'être of commit dddd60220f41 ("ipw2200: select
CFG80211_WEXT") was reverted in commit 2d36e008739e ("Revert "cfg80211:
make WEXT compatibility unselectable""). So revert this commit too.

Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
---
 drivers/net/wireless/ipw2x00/Kconfig | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Marcel Holtmann Jan. 3, 2015, 6:02 p.m. UTC | #1
Hi Paul,

> This reverts commit dddd60220f41775e634258efd1b54c6fa81ce706.
> 
> The raison d'être of commit dddd60220f41 ("ipw2200: select
> CFG80211_WEXT") was reverted in commit 2d36e008739e ("Revert "cfg80211:
> make WEXT compatibility unselectable""). So revert this commit too.
> 
> Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
> ---
> drivers/net/wireless/ipw2x00/Kconfig | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/ipw2x00/Kconfig b/drivers/net/wireless/ipw2x00/Kconfig
> index 21de4fe6cf2d..91c0cb3c368e 100644
> --- a/drivers/net/wireless/ipw2x00/Kconfig
> +++ b/drivers/net/wireless/ipw2x00/Kconfig
> @@ -65,8 +65,7 @@ config IPW2100_DEBUG
> 
> config IPW2200
> 	tristate "Intel PRO/Wireless 2200BG and 2915ABG Network Connection"
> -	depends on PCI && CFG80211
> -	select CFG80211_WEXT
> +	depends on PCI && CFG80211 && CFG80211_WEXT
> 	select WIRELESS_EXT
> 	select WEXT_SPY
> 	select WEXT_PRIV

why would you revert this? It is obviously the correct change to actually select CFG80211_WEXT.

Regards

Marcel

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Torvalds Jan. 3, 2015, 6:07 p.m. UTC | #2
On Sat, Jan 3, 2015 at 10:02 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
>
> why would you revert this? It is obviously the correct change to actually select CFG80211_WEXT.

I don't know about obvious, but yeah, I think the select in this case
is actually the better idea anyway.

We could make the CFG80211_WEXT help message be very negative so that
people aren't encouraged to select it even if they can, but then if
they need the ipw driver it gets selected because of that. Because the
ipw driver is probably the more important of the two if you just
happen to have old hardware but are upgrading yout software (and
anybody who recompiles their own kernel is obviously doing the
latter).

                         Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paul Bolle Jan. 3, 2015, 10:28 p.m. UTC | #3
On Sat, 2015-01-03 at 10:07 -0800, Linus Torvalds wrote:
> On Sat, Jan 3, 2015 at 10:02 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
> >
> > why would you revert this? It is obviously the correct change to actually select CFG80211_WEXT.
> 
> I don't know about obvious, but yeah, I think the select in this case
> is actually the better idea anyway.

Obviously it wasn't obvious to me!

My reasoning was that the "ipw2200: select CFG80211_WEXT" commit was
_solely_ a workaround for the breakage introduced by that other patch.
And since that one is now reverted the workaround wasn't needed anymore.

Besied, I thought we try to avoid select-ing symbols that can also be
set manually. As that makes it more likely to trigger circular
dependency problems in the kconfig tools, doesn't it?

> We could make the CFG80211_WEXT help message be very negative so that
> people aren't encouraged to select it even if they can, but then if
> they need the ipw driver it gets selected because of that. Because the
> ipw driver is probably the more important of the two if you just
> happen to have old hardware but are upgrading yout software (and
> anybody who recompiles their own kernel is obviously doing the
> latter).

Side note: am I correct in thinking that there's some successor to
CFG80211_WEXT and that the ipw2200 driver could, at least in theory, be
ported to that successor? (ipw2200 hardware appears to be a bit old, so
probably no one would care enough to actually do that.)
net/wireless/kconfig doesn't mention anything like that, so probably I'm
just confused.


Paul Bolle

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Johannes Berg Jan. 5, 2015, 10:05 a.m. UTC | #4
On Sat, 2015-01-03 at 23:28 +0100, Paul Bolle wrote:
> On Sat, 2015-01-03 at 10:07 -0800, Linus Torvalds wrote:
> > On Sat, Jan 3, 2015 at 10:02 AM, Marcel Holtmann <marcel@holtmann.org> wrote:
> > >
> > > why would you revert this? It is obviously the correct change to actually select CFG80211_WEXT.
> > 
> > I don't know about obvious, but yeah, I think the select in this case
> > is actually the better idea anyway.
> 
> Obviously it wasn't obvious to me!
> 
> My reasoning was that the "ipw2200: select CFG80211_WEXT" commit was
> _solely_ a workaround for the breakage introduced by that other patch.
> And since that one is now reverted the workaround wasn't needed anymore.

Well, you thought of it only as a workaround - but it makes sense. You
shouldn't have to 

> Besied, I thought we try to avoid select-ing symbols that can also be
> set manually. As that makes it more likely to trigger circular
> dependency problems in the kconfig tools, doesn't it?

I don't think that has much to do with whether or not they can be set
manually - it's more a question of what the dependencies of the selected
symbol are. In this case it's a bool leaf symbol with the only
dependency being something ipw already needs, so it's pretty much
guaranteed to be safe.

> Side note: am I correct in thinking that there's some successor to
> CFG80211_WEXT and that the ipw2200 driver could, at least in theory, be
> ported to that successor? (ipw2200 hardware appears to be a bit old, so
> probably no one would care enough to actually do that.)
> net/wireless/kconfig doesn't mention anything like that, so probably I'm
> just confused.

Well, see the big thread over there with the revert that I'm tempted to
not even read ...

The real successor, for various reasons (like simply always being able
to connect to a single AP, no matter how many others can be found by
scanning!), is nl80211.

Many drivers have been converted to the new framework by using mac80211,
but some drivers cannot use mac80211 because they do more in the
device/firmware. Among those are many of the old 11b/11g ones that still
exist in the tree, but also a few new ones (like mwifiex) that were
already written to cfg80211 APIs.

It should definitely be possible to convert all the drivers (*) to pure
cfg80211 instead of having direct userspace wext to driver calls - but
nobody has gone and done it.

johannes

(*) caveat - at least the playstation3 driver will require some new
nl80211 API

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jiri Kosina Jan. 5, 2015, 10:12 a.m. UTC | #5
On Mon, 5 Jan 2015, Johannes Berg wrote:

> Well, see the big thread over there with the revert that I'm tempted to
> not even read ...

I'd actually like to hear from you whether you share Emmanuel's point of 
view that my revert of your patch was inappropriate; I was really 
surprised that there might be even questions about that, given the obvious 
userspace wreckage.

Again, the only thing I really care for is the compatibility layer to 
userspace. I don't question at all the fact nl80211 provides much better 
functionality and its usage definitely should be encouraged.

Thanks,
Arend van Spriel Jan. 5, 2015, 10:14 a.m. UTC | #6
On 01/03/15 23:28, Paul Bolle wrote:
> On Sat, 2015-01-03 at 10:07 -0800, Linus Torvalds wrote:
>> On Sat, Jan 3, 2015 at 10:02 AM, Marcel Holtmann<marcel@holtmann.org>  wrote:
>>>
>>> why would you revert this? It is obviously the correct change to actually select CFG80211_WEXT.
>>
>> I don't know about obvious, but yeah, I think the select in this case
>> is actually the better idea anyway.
>
> Obviously it wasn't obvious to me!
>
> My reasoning was that the "ipw2200: select CFG80211_WEXT" commit was
> _solely_ a workaround for the breakage introduced by that other patch.
> And since that one is now reverted the workaround wasn't needed anymore.
>
> Besied, I thought we try to avoid select-ing symbols that can also be
> set manually. As that makes it more likely to trigger circular
> dependency problems in the kconfig tools, doesn't it?
>
>> We could make the CFG80211_WEXT help message be very negative so that
>> people aren't encouraged to select it even if they can, but then if
>> they need the ipw driver it gets selected because of that. Because the
>> ipw driver is probably the more important of the two if you just
>> happen to have old hardware but are upgrading yout software (and
>> anybody who recompiles their own kernel is obviously doing the
>> latter).
>
> Side note: am I correct in thinking that there's some successor to
> CFG80211_WEXT and that the ipw2200 driver could, at least in theory, be
> ported to that successor? (ipw2200 hardware appears to be a bit old, so
> probably no one would care enough to actually do that.)
> net/wireless/kconfig doesn't mention anything like that, so probably I'm
> just confused.

ipw2200 is a WEXT driver using some wext functionality (and struct 
wiphy) provided by cfg80211 hence it needs CFG80211_WEXT. I guess that 
is what makes it confusing.

Regards,
Arend


> Paul Bolle
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Johannes Berg Jan. 5, 2015, 10:20 a.m. UTC | #7
On Mon, 2015-01-05 at 11:12 +0100, Jiri Kosina wrote:
> On Mon, 5 Jan 2015, Johannes Berg wrote:
> 
> > Well, see the big thread over there with the revert that I'm tempted to
> > not even read ...
> 
> I'd actually like to hear from you whether you share Emmanuel's point of 
> view that my revert of your patch was inappropriate; I was really 
> surprised that there might be even questions about that, given the obvious 
> userspace wreckage.

Let's not pollute this thread with that though ... :-)

I agree that the revert was the right thing to do, but from what I saw
when I briefly glimpsed at part of the thread during my vacation (and
then deciding I didn't want to deal with it then) a lot has been said in
the thread that is completely wrong. I may not care enough to correct it
though.

johannes


--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paul Bolle Jan. 5, 2015, 5:38 p.m. UTC | #8
On Mon, 2015-01-05 at 11:14 +0100, Arend van Spriel wrote:
> On 01/03/15 23:28, Paul Bolle wrote:
> > Side note: am I correct in thinking that there's some successor to
> > CFG80211_WEXT and that the ipw2200 driver could, at least in theory, be
> > ported to that successor? (ipw2200 hardware appears to be a bit old, so
> > probably no one would care enough to actually do that.)
> > net/wireless/kconfig doesn't mention anything like that, so probably I'm
> > just confused.
> 
> ipw2200 is a WEXT driver using some wext functionality (and struct 
> wiphy) provided by cfg80211 hence it needs CFG80211_WEXT. I guess that 
> is what makes it confusing.

It doesn't help that I hardly know anything about mac80211, cfg80211 and
nl80211 (and lib80211 for that matter). To me these are mostly just
names that end in 80211.

Anyhow, concerning, CFG80211_WEXT: it seems the only functionality
provided by that symbol that ipw2200 uses directly is
cfg80211_wext_giwname(). Perhaps ipw2200 could have a private version of
that function, something like ipw2100's ipw2100_wx_get_name(). Should be
trivial to implement (ie, it could take _me_ a day or two).

But perhaps ipw2200 uses CFG80211_WEXT _indirectly_ too. Ie, in
net/wireless/core.c I stumbled on
    #ifdef CONFIG_CFG80211_WEXT
            rdev->wiphy.wext = &cfg80211_wext_handler;
    #endif


But I net/wireless/wext-core.c I then found
    #ifdef CONFIG_CFG80211_WEXT
            if (dev->ieee80211_ptr && dev->ieee80211_ptr->wiphy)
                    handlers = dev->ieee80211_ptr->wiphy->wext;
    #endif
    #ifdef CONFIG_WIRELESS_EXT
            if (dev->wireless_handlers)
                    handlers = dev->wireless_handlers;
    #endif

(There's much more to discover about WEXT, of course.) Anyhow, IPW2200
uses both CFG80211_WEXT and WIRELESS_EXT and cfg80211_wext_handler and
ipw2200's wireless_handlers appear to cover the same set of IOCTLS (one
exception: SIOCSIWPMKSA). So by now I'm really puzzled how this all fits
together.

Thanks,


Paul Bolle

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arend van Spriel Jan. 5, 2015, 6:22 p.m. UTC | #9
On 01/05/15 18:38, Paul Bolle wrote:
> On Mon, 2015-01-05 at 11:14 +0100, Arend van Spriel wrote:
>> On 01/03/15 23:28, Paul Bolle wrote:
>>> Side note: am I correct in thinking that there's some successor to
>>> CFG80211_WEXT and that the ipw2200 driver could, at least in theory, be
>>> ported to that successor? (ipw2200 hardware appears to be a bit old, so
>>> probably no one would care enough to actually do that.)
>>> net/wireless/kconfig doesn't mention anything like that, so probably I'm
>>> just confused.
>>
>> ipw2200 is a WEXT driver using some wext functionality (and struct
>> wiphy) provided by cfg80211 hence it needs CFG80211_WEXT. I guess that
>> is what makes it confusing.
>
> It doesn't help that I hardly know anything about mac80211, cfg80211 and
> nl80211 (and lib80211 for that matter). To me these are mostly just
> names that end in 80211.

Grapjas ;-)

cfg80211 provides thin-layer API for fullmac drivers (running 802.11 
stack on the device) and mac80211-based drivers (running 802.11 stack in 
kernel).

> Anyhow, concerning, CFG80211_WEXT: it seems the only functionality
> provided by that symbol that ipw2200 uses directly is
> cfg80211_wext_giwname(). Perhaps ipw2200 could have a private version of
> that function, something like ipw2100's ipw2100_wx_get_name(). Should be
> trivial to implement (ie, it could take _me_ a day or two).

Indeed or even an hour or two.

> But perhaps ipw2200 uses CFG80211_WEXT _indirectly_ too. Ie, in
> net/wireless/core.c I stumbled on
>      #ifdef CONFIG_CFG80211_WEXT
>              rdev->wiphy.wext =&cfg80211_wext_handler;
>      #endif

This is the "wext compatibility" being enabled for any cfg80211 or 
mac80211 based driver.

>
> But I net/wireless/wext-core.c I then found
>      #ifdef CONFIG_CFG80211_WEXT
>              if (dev->ieee80211_ptr&&  dev->ieee80211_ptr->wiphy)
>                      handlers = dev->ieee80211_ptr->wiphy->wext;
>      #endif

wext-core is the WEXT framework and here it extracts WEXT handlers from 
a cfg80211/mac80211-based driver that are store in wiphy structure.

>      #ifdef CONFIG_WIRELESS_EXT
>              if (dev->wireless_handlers)
>                      handlers = dev->wireless_handlers;
>      #endif

Here wext-core extracts WEXT handlers from a WEXT driver. struct 
net_device::wireless_handlers is only defined for CONFIG_WIRELESS_EXT.

> (There's much more to discover about WEXT, of course.) Anyhow, IPW2200
> uses both CFG80211_WEXT and WIRELESS_EXT and cfg80211_wext_handler and
> ipw2200's wireless_handlers appear to cover the same set of IOCTLS (one
> exception: SIOCSIWPMKSA). So by now I'm really puzzled how this all fits
> together.

I think ipw2200 is a bit of both worlds indeed adopting the use of 
struct wiphy and wiphy_register() call. That seems to suggest it is a 
cfg80211 driver, but it does not register any cfg80211 driver callbacks 
(see libipw_config_ops in libipw_module.c). So it overrides the WEXT 
ioctls because it needs that to interact with the device.

Regards,
Arend

> Thanks,
>
>
> Paul Bolle
>

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Johannes Berg Jan. 5, 2015, 6:57 p.m. UTC | #10
On Mon, 2015-01-05 at 18:38 +0100, Paul Bolle wrote:

> > ipw2200 is a WEXT driver using some wext functionality (and struct 
> > wiphy) provided by cfg80211 hence it needs CFG80211_WEXT. I guess that 
> > is what makes it confusing.
> 
> It doesn't help that I hardly know anything about mac80211, cfg80211 and
> nl80211 (and lib80211 for that matter). To me these are mostly just
> names that end in 80211.

:-)
There isn't really all that much that ipw2x00 is using from cfg80211
though - of note is that it has completely empty ops:

static struct cfg80211_ops libipw_config_ops = { };

IOW - all it does is register with the framework - courtesy of
a3caa99e6c68f. It's practically useless.

> Anyhow, concerning, CFG80211_WEXT: it seems the only functionality
> provided by that symbol that ipw2200 uses directly is
> cfg80211_wext_giwname(). Perhaps ipw2200 could have a private version of
> that function, something like ipw2100's ipw2100_wx_get_name(). Should be
> trivial to implement (ie, it could take _me_ a day or two).

We could just revert that part of the commit above - or even completely.

However, in theory at least doing *that* would now be a userspace
regression - today you can at least discover the presence of ipw2200
devices with nl80211, even if you can't do anything with them that way.

> But perhaps ipw2200 uses CFG80211_WEXT _indirectly_ too. Ie, in
> net/wireless/core.c I stumbled on
>     #ifdef CONFIG_CFG80211_WEXT
>             rdev->wiphy.wext = &cfg80211_wext_handler;
>     #endif

I don't think it does - see the note about the ops above. If it did,
it'd have to implement the ops.

> But I net/wireless/wext-core.c I then found
>     #ifdef CONFIG_CFG80211_WEXT
>             if (dev->ieee80211_ptr && dev->ieee80211_ptr->wiphy)
>                     handlers = dev->ieee80211_ptr->wiphy->wext;
>     #endif
>     #ifdef CONFIG_WIRELESS_EXT
>             if (dev->wireless_handlers)
>                     handlers = dev->wireless_handlers;
>     #endif
> 
> (There's much more to discover about WEXT, of course.) Anyhow, IPW2200
> uses both CFG80211_WEXT and WIRELESS_EXT and cfg80211_wext_handler and
> ipw2200's wireless_handlers appear to cover the same set of IOCTLS (one
> exception: SIOCSIWPMKSA). So by now I'm really puzzled how this all fits
> together.

Well, this was meant as a transition mechanism for drivers. Ultimately,
the way we thought how you'd convert a driver (and how we converted
mac80211) would be to have the wext handlers like for example the scan
ones:

static iw_handler ipw_wx_handlers[] = {
...
        IW_HANDLER(SIOCSIWSCAN, ipw_wx_set_scan),
        IW_HANDLER(SIOCGIWSCAN, ipw_wx_get_scan),
...
};

Then you could make a patch that uses the cfg80211 APIs for scanning in
the driver -- i.e. implement the cfg80211_ops.scan method, report frames
to the cfg80211 scanning and remove all the ieee->network_list stuff
etc. using the related cfg80211 API (e.g. cfg80211_get_bss() and friends
for getting a network, etc.) And then you'd change the handlers to be

static iw_handler ipw_wx_handlers[] = {
...
        IW_HANDLER(SIOCSIWSCAN, cfg80211_wext_siwscan),
        IW_HANDLER(SIOCGIWSCAN, cfg80211_wext_giwscan),
...
};

This part would have to be done in a single patch.

Multiple other groups of ioctls could be converted in similar patches,
until at the end you can completely remove ipw_wx_handlers and rely
entirely on cfg80211's wext compatibility.

So far the theory - in practice nobody cared enough to start working on
any of these drivers, let alone actually has the hardware today.

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paul Bolle Jan. 5, 2015, 10:05 p.m. UTC | #11
On Mon, 2015-01-05 at 19:57 +0100, Johannes Berg wrote:
> Multiple other groups of ioctls could be converted in similar patches,
> until at the end you can completely remove ipw_wx_handlers and rely
> entirely on cfg80211's wext compatibility.
> 
> So far the theory - in practice nobody cared enough to start working on
> any of these drivers, let alone actually has the hardware today.

So my suggestion to make ipw2200 no longer use cfg80211_wext_giwname()
would actually be backwards. What's actually needed, in theory, is to
use more of what's provided under CFG80211_WEXT (and, I guess, less of
what's provided under WIRELESS_EXT). Did I get that right?

Thanks,


Paul Bolle

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arend van Spriel Jan. 5, 2015, 10:13 p.m. UTC | #12
On 01/05/15 23:05, Paul Bolle wrote:
> On Mon, 2015-01-05 at 19:57 +0100, Johannes Berg wrote:
>> Multiple other groups of ioctls could be converted in similar patches,
>> until at the end you can completely remove ipw_wx_handlers and rely
>> entirely on cfg80211's wext compatibility.
>>
>> So far the theory - in practice nobody cared enough to start working on
>> any of these drivers, let alone actually has the hardware today.
>
> So my suggestion to make ipw2200 no longer use cfg80211_wext_giwname()
> would actually be backwards. What's actually needed, in theory, is to
> use more of what's provided under CFG80211_WEXT (and, I guess, less of
> what's provided under WIRELESS_EXT). Did I get that right?

Yes, but as Johannes indicated it needs consideration what to group in 
the patches.

Regards,
Arend

> Thanks,
>
>
> Paul Bolle
>

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kalle Valo Jan. 6, 2015, 8:03 a.m. UTC | #13
Paul Bolle <pebolle@tiscali.nl> writes:

> This reverts commit dddd60220f41775e634258efd1b54c6fa81ce706.
>
> The raison d'être of commit dddd60220f41 ("ipw2200: select
> CFG80211_WEXT") was reverted in commit 2d36e008739e ("Revert "cfg80211:
> make WEXT compatibility unselectable""). So revert this commit too.
>
> Signed-off-by: Paul Bolle <pebolle@tiscali.nl>

Based on the discussion I have dropped this patch from my queue.
Johannes Berg Jan. 6, 2015, 8:19 a.m. UTC | #14
On Mon, 2015-01-05 at 23:13 +0100, Arend van Spriel wrote:
> On 01/05/15 23:05, Paul Bolle wrote:
> > On Mon, 2015-01-05 at 19:57 +0100, Johannes Berg wrote:
> >> Multiple other groups of ioctls could be converted in similar patches,
> >> until at the end you can completely remove ipw_wx_handlers and rely
> >> entirely on cfg80211's wext compatibility.
> >>
> >> So far the theory - in practice nobody cared enough to start working on
> >> any of these drivers, let alone actually has the hardware today.
> >
> > So my suggestion to make ipw2200 no longer use cfg80211_wext_giwname()
> > would actually be backwards. What's actually needed, in theory, is to
> > use more of what's provided under CFG80211_WEXT (and, I guess, less of
> > what's provided under WIRELESS_EXT). Did I get that right?
> 
> Yes, but as Johannes indicated it needs consideration what to group in 
> the patches.

Oh, that's not strictly necessary - that was just so it would actually
work after each single patch. If you wanted to, you could do it all in a
single huge patch as well :-) I was explaining though why we did the
cfg80211-wext code the way it is - which enables making smaller changes
that don't break the driver.

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Johannes Berg Jan. 6, 2015, 8:23 a.m. UTC | #15
On Mon, 2015-01-05 at 23:05 +0100, Paul Bolle wrote:
> On Mon, 2015-01-05 at 19:57 +0100, Johannes Berg wrote:
> > Multiple other groups of ioctls could be converted in similar patches,
> > until at the end you can completely remove ipw_wx_handlers and rely
> > entirely on cfg80211's wext compatibility.
> > 
> > So far the theory - in practice nobody cared enough to start working on
> > any of these drivers, let alone actually has the hardware today.
> 
> So my suggestion to make ipw2200 no longer use cfg80211_wext_giwname()
> would actually be backwards. What's actually needed, in theory, is to
> use more of what's provided under CFG80211_WEXT (and, I guess, less of
> what's provided under WIRELESS_EXT). Did I get that right?

Yes, though I'd argue there are multiple levels of truth here.

Yours is the theoretical, hopefully-forward-looking one where we still
expect the driver to actually be modified to take advantage of the new
frameworks (which is independent of wext support towards userspace). In
that scenario, yes, it should use more until it uses all, and then it
can stop concerning itself with wext (which would be a win because
driver/wext interaction is always finicky) (*)

Then there's the other level that you were looking at earlier - simply
removing all of this again from this driver because nobody is going to
work on it. That'd actually make sense and shrink the driver footprint
(no need to load cfg80211 for using almost nothing of it) but this is no
longer feasible since it would again break userspace API - as I said you
can today discover the presence of this device/driver with nl80211 -
that would be broken by removing cfg80211 from here.

And then there's the practical third version that's likely to happen -
i.e. nothing :-)

johannes

(*) FWIW, if done to all drivers it would allow shrinking cfg80211-wext
by a lot because all the EXPORT_SYMBOL would no longer be needed - but
come to think of it we can fix that differently.

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Johannes Berg Jan. 6, 2015, 1:26 p.m. UTC | #16
On Tue, 2015-01-06 at 09:23 +0100, Johannes Berg wrote:

> Yours is the theoretical, hopefully-forward-looking one where we still
> expect the driver to actually be modified to take advantage of the new
> frameworks (which is independent of wext support towards userspace). In
> that scenario, yes, it should use more until it uses all, and then it
> can stop concerning itself with wext (which would be a win because
> driver/wext interaction is always finicky) (*)

I just realized that in order to actually do this you'd have to revert
04b0c5c6995103eef56391c163e970ab1f03b59f first, but that would be pretty
simple.

johannes

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/wireless/ipw2x00/Kconfig b/drivers/net/wireless/ipw2x00/Kconfig
index 21de4fe6cf2d..91c0cb3c368e 100644
--- a/drivers/net/wireless/ipw2x00/Kconfig
+++ b/drivers/net/wireless/ipw2x00/Kconfig
@@ -65,8 +65,7 @@  config IPW2100_DEBUG
 
 config IPW2200
 	tristate "Intel PRO/Wireless 2200BG and 2915ABG Network Connection"
-	depends on PCI && CFG80211
-	select CFG80211_WEXT
+	depends on PCI && CFG80211 && CFG80211_WEXT
 	select WIRELESS_EXT
 	select WEXT_SPY
 	select WEXT_PRIV