On Wed, 2012-08-08 at 14:53 +0300, Victor Goldenshtein wrote:
> + * @NL80211_FEATURE_DFS: Radar detection is supported in the HW/driver.
How will you know what kind of radar detection is supported? That is, HT
20, HT 40, in the future VHT80/160/80+80?
> + int (*start_radar_detection)(struct wiphy *wiphy,
> + struct net_device *dev,
> + struct ieee80211_channel *chan);
Channel type/bandwidth might be needed?
> +void cfg80211_radar_detected(struct net_device *dev,
> + struct ieee80211_channel *chan, gfp_t gfp);
Is the channel parameter useful? Only one detection can be triggered at
any given time.
> void cfg80211_send_rx_auth(struct net_device *dev, const u8 *buf, size_t len)
> {
> struct wireless_dev *wdev = dev->ieee80211_ptr;
> @@ -1006,3 +1008,39 @@ bool cfg80211_rx_unexpected_4addr_frame(struct net_device *dev,
> return nl80211_unexpected_4addr_frame(dev, addr, gfp);
> }
> EXPORT_SYMBOL(cfg80211_rx_unexpected_4addr_frame);
> +
> +int cfg80211_start_radar_detection(struct cfg80211_registered_device *rdev,
> + struct net_device *dev,
> + struct ieee80211_channel *chan)
> +{
> + int err;
> +
> + if (!rdev->ops->start_radar_detection)
> + return -EOPNOTSUPP;
> +
> + err = rdev->ops->start_radar_detection(&rdev->wiphy, dev, chan);
> + if (!err)
> + chan->radar_detect_timeout = jiffies +
> + msecs_to_jiffies(IEEE80211_DFS_MIN_CAC_TIME_MS);
> + else {
> + chan->radar_detect_timeout = 0;
> + chan->cac_type = 0;
> + }
You're not setting cac_type in the good case, and also
radar_detect_timeout can actually be 0 in the good case due to jiffies
wrap. How is that handled?
> + chan->cac_type = 0;
Also here and above you should use channel type enums. In fact, 0 is
"NOHT". Oops. Need a boolean I guess that indicates validity.
> + bool dfs_supported = (rdev->wiphy.features & NL80211_FEATURE_DFS);
why bother with the variable?
> + if (!chan || !(chan->flags & IEEE80211_CHAN_RADAR))
> + return -EINVAL;
> +
> + if (chan->cac_type)
> + return -EBUSY;
> +
> + chan->cac_type = cac_type;
Aha. But these things probably should be in the function:
> + return cfg80211_start_radar_detection(rdev, dev, chan);
so it's actually a potentially useful function for other places.
Otherwise you could just manually inline it here, I see no issues with
that either.
johannes
--
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
