2009-07-28 16:28:02

by Dave Kilroy

[permalink] [raw]
Subject: Re: [PATCH] orinoco: enable cfg80211 "set_channel" operation

Holger Schurig wrote:
> Signed-off-by: Holger Schurig <[email protected]>

Cool.

> + if (orinoco_lock(priv, &flags) != 0)
> + return -EBUSY;
> +
> + priv->channel = channel;
> + if (priv->iw_mode == NL80211_IFTYPE_MONITOR) {
> + /* Fast channel change - no commit if successful */
> + hermes_t *hw = &priv->hw;
> + err = hermes_docmd_wait(hw, HERMES_CMD_TEST |
> + HERMES_TEST_SET_CHANNEL,
> + channel, NULL);
> + }
> + orinoco_unlock(priv, &flags);
> +
> + return err;
> +}

Looks right for monitor mode (as you tested), but for ad-hoc the channel
change would be delayed until the next SIOCSIWCOMMIT. Which for cfg80211
is currently only on change_vif.

Adding a call to orinoco_commit(priv) in the ad-hoc case should push the
change to the card. I think we'll end up with this call in each of the
orinoco cfg80211 functions. Otherwise we have to work out how to do the
changes incrementally across the different hw/fw.

You can also eliminate orinoco_ioctl_setfreq from wext.c with this change.


Thanks,

Dave.


2009-07-29 07:16:51

by Holger Schurig

[permalink] [raw]
Subject: Re: [PATCH] orinoco: enable cfg80211 "set_channel" operation

On Tuesday 28 July 2009 18:27:54 Dave wrote:
> You can also eliminate orinoco_ioctl_setfreq from wext.c with
> this change.

You mean this ?!?

--- linux-wl.orig/drivers/net/wireless/orinoco/wext.c
+++ linux-wl/drivers/net/wireless/orinoco/wext.c
@@ -372,55 +372,6 @@
return 0;
}

-static int orinoco_ioctl_setfreq(struct net_device *dev,
- struct iw_request_info *info,
- struct iw_freq *frq,
- char *extra)
-{
- struct orinoco_private *priv = ndev_priv(dev);
- int chan = -1;
- unsigned long flags;
- int err = -EINPROGRESS; /* Call commit handler */
-
- /* In infrastructure mode the AP sets the channel */
- if (priv->iw_mode == NL80211_IFTYPE_STATION)
- return -EBUSY;
-
- if ((frq->e == 0) && (frq->m <= 1000)) {
- /* Setting by channel number */
- chan = frq->m;
- } else {
- /* Setting by frequency */
- int denom = 1;
- int i;
-
- /* Calculate denominator to rescale to MHz */
- for (i = 0; i < (6 - frq->e); i++)
- denom *= 10;
-
- chan = ieee80211_freq_to_dsss_chan(frq->m / denom);
- }
-
- if ((chan < 1) || (chan > NUM_CHANNELS) ||
- !(priv->channel_mask & (1 << (chan-1))))
- return -EINVAL;
-
- if (orinoco_lock(priv, &flags) != 0)
- return -EBUSY;
-
- priv->channel = chan;
- if (priv->iw_mode == NL80211_IFTYPE_MONITOR) {
- /* Fast channel change - no commit if successful */
- hermes_t *hw = &priv->hw;
- err = hermes_docmd_wait(hw, HERMES_CMD_TEST |
- HERMES_TEST_SET_CHANNEL,
- chan, NULL);
- }
- orinoco_unlock(priv, &flags);
-
- return err;
-}
-
static int orinoco_ioctl_getfreq(struct net_device *dev,
struct iw_request_info *info,
struct iw_freq *frq,
@@ -1481,7 +1432,6 @@
static const iw_handler orinoco_handler[] = {
STD_IW_HANDLER(SIOCSIWCOMMIT, orinoco_ioctl_commit),
STD_IW_HANDLER(SIOCGIWNAME, cfg80211_wext_giwname),
- STD_IW_HANDLER(SIOCSIWFREQ, orinoco_ioctl_setfreq),
STD_IW_HANDLER(SIOCGIWFREQ, orinoco_ioctl_getfreq),
STD_IW_HANDLER(SIOCSIWMODE, cfg80211_wext_siwmode),
STD_IW_HANDLER(SIOCGIWMODE, cfg80211_wext_giwmode),

I tried, but with then an "iwconfig eth1 channel 1" gave me an
error. I did not investigate this further.

--
http://www.holgerschurig.de

2009-07-28 16:30:32

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] orinoco: enable cfg80211 "set_channel" operation

On Tue, 2009-07-28 at 17:27 +0100, Dave wrote:

> Looks right for monitor mode (as you tested), but for ad-hoc the channel
> change would be delayed until the next SIOCSIWCOMMIT. Which for cfg80211
> is currently only on change_vif.

You won't be getting this call for ad-hoc mode, because then the channel
is contained in the ibss_join() parameters.

johannes


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part

2009-07-28 20:43:20

by Dave Kilroy

[permalink] [raw]
Subject: Re: [PATCH] orinoco: enable cfg80211 "set_channel" operation

Johannes Berg wrote:
> On Tue, 2009-07-28 at 18:29 +0200, Johannes Berg wrote:
>> On Tue, 2009-07-28 at 17:27 +0100, Dave wrote:
>>
>>> Looks right for monitor mode (as you tested), but for ad-hoc the channel
>>> change would be delayed until the next SIOCSIWCOMMIT. Which for cfg80211
>>> is currently only on change_vif.
>> You won't be getting this call for ad-hoc mode, because then the channel
>> is contained in the ibss_join() parameters.
>
>
>>> You can also eliminate orinoco_ioctl_setfreq from wext.c with this
>>> change.
>
> That also means you can't eliminate that, of course, since for connect()
> the channels is also contained in the parameters, and the set_freq isn't
> invoked.

Ah. Thanks for the corrections.


Dave.

2009-07-30 18:47:22

by Dave Kilroy

[permalink] [raw]
Subject: Re: [PATCH] orinoco: enable cfg80211 "set_channel" operation

Holger Schurig wrote:
> On Tuesday 28 July 2009 18:27:54 Dave wrote:
>> You can also eliminate orinoco_ioctl_setfreq from wext.c with
>> this change.
>
> You mean this ?!?

Yes, except:

> @@ -1481,7 +1432,6 @@
> static const iw_handler orinoco_handler[] = {
> STD_IW_HANDLER(SIOCSIWCOMMIT, orinoco_ioctl_commit),
> STD_IW_HANDLER(SIOCGIWNAME, cfg80211_wext_giwname),
> - STD_IW_HANDLER(SIOCSIWFREQ, orinoco_ioctl_setfreq),
+ STD_IW_HANDLER(SIOCSIWFREQ, cfg80211_wext_setfreq ish),

> STD_IW_HANDLER(SIOCGIWFREQ, orinoco_ioctl_getfreq),
> STD_IW_HANDLER(SIOCSIWMODE, cfg80211_wext_siwmode),
> STD_IW_HANDLER(SIOCGIWMODE, cfg80211_wext_giwmode),
>
> I tried, but with then an "iwconfig eth1 channel 1" gave me an
> error. I did not investigate this further.

And as Johannes pointed out, we need to have the non-cfg80211 version so
adhoc connections can still be made via wext.

So the original patch is fine as is. Ack from me.


Dave.

2009-07-28 16:32:11

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] orinoco: enable cfg80211 "set_channel" operation

On Tue, 2009-07-28 at 18:29 +0200, Johannes Berg wrote:
> On Tue, 2009-07-28 at 17:27 +0100, Dave wrote:
>
> > Looks right for monitor mode (as you tested), but for ad-hoc the channel
> > change would be delayed until the next SIOCSIWCOMMIT. Which for cfg80211
> > is currently only on change_vif.
>
> You won't be getting this call for ad-hoc mode, because then the channel
> is contained in the ibss_join() parameters.


> > You can also eliminate orinoco_ioctl_setfreq from wext.c with this
> > change.

That also means you can't eliminate that, of course, since for connect()
the channels is also contained in the parameters, and the set_freq isn't
invoked.

johannes


Attachments:
signature.asc (801.00 B)
This is a digitally signed message part