On Mon, 2011-11-28 at 17:12 +0100, Daniel Golle wrote:
> This adds support for antenna switch configuration profiles to nl80211.
Meh, I couldn't find this patch because the subjects are all wrong.
Three patches with exactly the same subject?! Come on. Also please use
threaded mode to submit -- otherwise the 0/N discussion & the patches
get torn apart (which is the reason I'm replying here).
Anyway, enough complaining.
> /**
> + * struct wiphy_extant_profile - User-parsable external antenna switch info
> + *
> + * This structure is used to provide the available switch configurations.
> + * @id: unique identifier of the profile
> + * @name: profile name (a short string)
> + * @description: user-parsable description of the profile
> + */
> +struct wiphy_extant_profile {
> + int id;
> + char *name;
> + char *desc;
const
> + struct wiphy_extant_profile *extant;
const
Also, shouldn't this allow multiple profiles?
It's a bit strange to have a profile ID but then only a single
profile ...
> +/* policy for external antenna switch configuration profiles */
> +static const struct nla_policy
> +nl80211_easp_policy[NL80211_EASP_ATTR_MAX + 1] = {
> + [NL80211_EASP_ATTR_ID] = { .type = NLA_U32 },
> + [NL80211_EASP_ATTR_NAME] = { .type = NLA_STRING,
> + .len = IEEE80211_MAX_DATA_LEN },
> + [NL80211_EASP_ATTR_DESC] = { .type = NLA_STRING,
> + .len = IEEE80211_MAX_DATA_LEN },
> +};
Those length restrictions are completely bogus, but they're also
unnecessary since this data is output data only so you aren't using the
policy anywhere anyway.
> @@ -781,6 +796,48 @@ static int nl80211_send_wiphy(struct sk_buff *msg, u32 pid, u32 seq, int flags,
> }
> }
>
> + /*
> + * antenna switch configuration profile support
> + */
> + if (dev->wiphy.flags & WIPHY_FLAG_HAS_EXTANT_SWITCH) {
> + struct wiphy_extant_profile *weprof;
> + nl_extant = nla_nest_start(msg, NL80211_ATTR_WIPHY_EXTANT);
> + if (!nl_extant)
> + goto nla_put_failure;
> +
> + /* currently selected configuration profile */
> + if (dev->ops->get_extant) {
> + u32 extant;
> + int res;
> + res = dev->ops->get_extant(&dev->wiphy, &extant);
> + if (!res)
> + NLA_PUT_U32(msg,
> + NL80211_EXTANT_ATTR_STATE,
> + extant);
> + }
> +
> + /* available profiles */
> + i=0;
> + nl_easps = nla_nest_start(msg, NL80211_EXTANT_ATTR_PROFILES);
> + for (weprof = dev->wiphy.extant; weprof->id >= 0; weprof++) {
Oh so that's supposed to be an array -- there's no way to tell this from
the header files. It's acceptable, but please give it an explicit
length.
Anyway, apart from these technical issues that you must fix, I also want
somebody who knows what this is used for and how it is used to review
it, which might mean waiting for Adrian who says it'll be a couple of
weeks at least.
johannes
--------------------------------------------------------------------------------------
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Douglas Lusk, Peter Gleissner, Hannes Schwaderer
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052
��.n��������+%������w��{.n�����{���zW����ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
