Re: [PATCHv2 3/4] cfg80211: Add Support set btcoex priority value

From: Johannes Berg <johannes@sipsolutions.net>
To: c_traja@qti.qualcomm.com, linux-wireless@vger.kernel.org
Cc: ath10k@lists.infradead.org
Subject: Re: [PATCHv2 3/4] cfg80211: Add Support set btcoex priority value
Date: Thu, 02 Mar 2017 09:50:59 +0100	[thread overview]
Message-ID: <1488444659.8390.8.camel@sipsolutions.net> (raw)
In-Reply-To: <1488029873-14600-4-git-send-email-c_traja@qti.qualcomm.com>

Is there much point in having 4 rather than just 2 patches?

> +	int     (*set_btcoex)(struct wiphy *wiphy, bool enabled,
> +			      int btcoex_priority);

Shouldn't that be u32 as a bitmap?

> +	bool btcoex_priority_support;

Why not use an extended nl80211 feature flag directly?

> + * @NL80211_ATTR_BTCOEX_PRIORITY: This is for the driver which
> + *     support btcoex priority feature. It used with
> %NL80211_CMD_SET_BTCOEX.
> + *     This will have u32 BITMAP value which represents
> + *     frame(bk, be, vi, vo, mgmt, beacon) type and that will have
> more
> + *     priority than a BT traffic.

I think you need to define the bits somewhere in an enum - i.e. which
one is VO, VI, ...

> +	int btcoex_priority = -1;

That -1 is pretty useless, if the driver doesn't support it, hopefully
it won't look at the value at all?

johannes