On Mon, 2011-12-19 at 17:06 +0100, Johannes Berg wrote:
> On Mon, 2011-12-19 at 13:17 +0200, Eliad Peller wrote:
>
> > >> diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
> > >> index c305841..82fc318 100644
> > >> --- a/drivers/net/wireless/wl12xx/main.c
> > >> +++ b/drivers/net/wireless/wl12xx/main.c
> > >> @@ -2269,6 +2269,17 @@ out:
> > >> cancel_work_sync(&wl->recovery_work);
> > >> }
> > >>
> > >> +static int wl12xx_op_change_interface(struct ieee80211_hw *hw,
> > >> + struct ieee80211_vif *vif,
> > >> + enum nl80211_iftype new_type, bool p2p)
> > >> +{
> > >> + wl1271_op_remove_interface(hw, vif);
> > >> +
> > >> + vif->type = ieee80211_iftype_p2p(new_type, p2p);
> > >> + vif->p2p = p2p;
> > >> + return wl1271_op_add_interface(hw, vif);
> > >> +}
> > >> +
> > >
> > > As Arik already commented, this looks a bit weird. Shouldn't the type
> > > and p2p elements be changed in mac80211 itself? What about doing so if
> > > the op_change_interface returns success? Currently, the opposite seems
> > > to happen (ie. mac80211 changes type back to the original vif.type if
> > > the op fails).
> > >
> > iwlegacy and ath9k are also setting vif->type and vif->p2p in a similar manner.
> > add_interface(vif) assumes vif->type and vif->p2p are correct, so it
> > makes sense to update the vif before adding it.
> > i also think this behavior is a bit ugly, but this is the current
> > mac80211 implementation, and i don't see a good enough reason to
> > change it :)
>
> The reason I did it this way is because it simplified the code in the
> driver (for me at least) -- I could call functions that work on a vif
> and look at vif->type/p2p before and after doing my own changes, whereas
> if mac80211 was doing the modification I'd have to add extra arguments
> everywhere.
Yeah, in our driver it's simpler to do it like this too. It's a bit
weird, but I don't think it's a problem. And now that one, two, three
drivers do it the same way, it has become the de-facto way of doing
it. ;)
Would deserve a comment somewhere, IMHO, though.
--
Cheers,
Luca.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
